fix(thumbs): WPB-21131 generate thumbs with the exif orientation #686
Conversation
Summary: This commit removes the custom handling of EXIF orientation from the JavaScript components, simplifying the rendering logic. It adds support in Go code for resolving the orientation based on the EXIF tag. Detailed Changes: - frontend/assets/editor.diaporama/res/js/editor.js: - Removed all logic related to handling `image_exif_orientation` metadata for styling image elements. - frontend/assets/editor.diaporama/res/js/preview.js: - Removed EXIF orientation handling logic for image preview adjustments. - go.mod: - Added `github.com/disintegration/imageorient` package for handling image orientation based on EXIF metadata. - go.sum: - Updated checksums corresponding to the new Go module additions. - scheduler/actions/images/encoding/codec.go: - Implemented the use of `imageorient.Decode` to handle image orientation based on EXIF metadata in the image decoding process. - scheduler/actions/images/encoding/codec_test.go: - Added test to verify orientation handling in the decoding process.
| "io" | ||
|
|
||
| "github.com/disintegration/imageorient" | ||
| "github.com/disintegration/imaging" |
There was a problem hiding this comment.
Maybe we can replace this library as a next step
| }} | ||
| />); | ||
|
|
||
| } No newline at end of file |
There was a problem hiding this comment.
We could introduce prettier to enforce removal of the trailing spaces
cristianoliveira
force-pushed
the
fix/exif-thumb-orientation-with-lib
branch
from
89706d7 to
6590239
Compare
cristianoliveira
force-pushed
the
fix/exif-thumb-orientation-with-lib
branch
from
6590239 to
ac3b57c
Compare
|
Looks good! Demooutput.mp4 |
There was a problem hiding this comment.
I don't agree here : old thumbnails will still need the CSS fix!
=> We should let all that code here, and in the backend, just ensure that we do not set this metadata anymore for new ones (EXIF extraction job)
There was a problem hiding this comment.
That makes sense
I changed to stop injecting the exif orientation for now on, so the new uploads don't contain it, while the old that contains the metadata, the css rotation is applied
Demo
output.mp4
|
|
||
| // Decode reads an image from the provided reader | ||
| func (c defaultCodec) Decode(reader io.Reader) (image.Image, error) { | ||
| return imaging.Decode(reader) |
There was a problem hiding this comment.
Sorry to bother again :-)
Does this have any perf implication for formats that do not support exif ? It seems that exif is JPG/TIFF only, maybe we should use the "standard" decode for BMP, PNG, WEBP...?
There was a problem hiding this comment.
There might be a minor performance hit. For our case I believe is better to configure it upon decoder instantiation, to avoid having non-intuitive rotation only for one format type. Something like:
t.codec = encoding.NewImageCodec(fileFormat, &encoding.CodecOptions{
EnforceExifOrientation: true,
})Then in case is not enforced it uses the standard encoding
There was a problem hiding this comment.
I notice also that there are some img types that simply don't support exif at all, for instance, GIF / BMP. I see the underlining library handle these already
2 participants
Summary:
This commit removes the custom handling of EXIF orientation from the JavaScript components, simplifying the rendering logic. It adds support in Go code for resolving the orientation based on the EXIF tag.
Detailed Changes:
imageorient.Decodeto handle image orientation based on EXIF metadata in the image decoding process.image_exif_orientationinjection for new uploadsDEPS
github.com/disintegration/imageorientpackage for handling image orientation based on EXIF metadata.How to check
ort-rotate-xSample:
orientation.zip