EXIF rotation is not reflected in Skia port

[mpsuzuki]

Skia framework does not reflect the rotation info in EXIF of JPEG automatically. This sample SVG
jpg-color-gray.zip
is rendered like this:

The expected result (by my latest Cairo port) is looking like this:

This issue seems to be well known by Skia programmers, SkiaSharp Issue #836, flutter Issue #10533. In fact, there was a mention sounding as if it would be available in future version of Skia skia-discuss forum message in 2016.

I would try to include auto-orientation feature in Skia port.

[mpsuzuki]

Yet I've not confirmed about the memory leaks, 5ca95c3 would improve the situation. The resulted PNG is looking like this:

Unfortunately, inclusion of SkCodec.h (to work with EXIF) causes some warning by compiler, like

[  7%] Building CXX object CMakeFiles/SVGNativeViewerLib.dir/ports/skia/SkiaSVGRenderer.cpp.o
In file included from /tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/../private/SkEnco
dedInfo.h:13:0,
                 from /tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/SkCodec.h:13,
                 from /tmp/svg-native-viewer/svgnative/ports/skia/SkiaSVGRenderer.cpp:17:
/tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/../private/../../third_party/skcms/skcm
s.h:51:5: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
     };
     ^
/tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/../private/../../third_party/skcms/skcm
s.h:56:5: warning: ISO C++ prohibits anonymous structs [-Wpedantic]
     };
     ^
In file included from /tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/SkCodec.h:14:0,
                 from /tmp/svg-native-viewer/svgnative/ports/skia/SkiaSVGRenderer.cpp:17:
/tmp/svg-native-viewer/svgnative/../third_party/skia/include/codec/SkCodecAnimation.h:42:2: warning: extra
 ‘;’ [-Wpedantic]
 };
  ^
[ 15%] Linking CXX static library libSVGNativeViewerLib.a

I'm wondering how to fix these warning...

[dirkschulze]

@mpsuzuki I assume compiling of Skia worked. Could that be a difficulty with the svg-native-viewer project settings that are in conflict with the settings of Skia?

[mpsuzuki]

Oh, I apologize making you confused. Yes, compiling Skia itself is successfully finished. But the inclusion of skcms.h causes the warning about the nameless structures in the union. It's just warning, and compiling itself finishes successfully. Maybe, giving some compiler flags (to compile SkiaSVGRenderer.cpp) would be able to calm it, but I'm wondering whether it's the way to go.

Checking the latest source at skia.googlesource.com, still it uses nameless structure.

// Unified representation of 'curv' or 'para' tag data, or a 1D table from 'mft1' or 'mft2'
typedef union skcms_Curve {
    struct {
        uint32_t alias_of_table_entries;
        skcms_TransferFunction parametric;
    };
    struct {
        uint32_t table_entries;
        const uint8_t* table_8;
        const uint8_t* table_16;
    };
} skcms_Curve;

How do you think about this coding style? It should be permitted only for the external software (and SVG Native Viewer's own code should not permit this style)?

[dirkschulze]

@mpsuzuki Ultimately, fixing the issue in Skia and providing a PR for the Skia project seems to be the best approach. In the short term I we shouldn’t change or require changes in the Skia source code itself. If the above can be added to svg-native-viewer it is fine as well. But take the long term maintenance effort into account. The Skia source code is changing all the time and the above union might no longer work with newer Skia code if we add it to svg-native-viewer. Maybe ignoring the warning for Skia subdirectories might be the easiest and more long term approach after all.

[mpsuzuki]

@dirkschulze Thanks, I found this article, and I understand as an anonymous structure in a union is not problematic C11 (not C++11). So Skia would not change about this (until someday in the future they care about legacy compilers).

Unfortunately, we cannot specify both of the std versions of C and C++ at the same time, so, the only available option for us is "-Wno-pedantic". Maybe future C++ would permit such anonymous structure in an union, but requesting newer C++ std is reverting your past effort to accept older C++ compiler (and I don't want to do that).

It is possible for cmake to set special compiler options for specific sources (like this), so I would add -Wno-pedantic for SkiaSVGRenderer.cpp, like b229d2c

If anybody thinks as "if so, please separate the rotation function to another file, to minimize the effect of the special compiler option", please let me know, I can do that easily.

[mpsuzuki]

Just I've made a pull request #56, please review.

[mpsuzuki]

@dirkschulze Thanks! here I close this issue.