Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Add an additional/alternative auto-image-rotate option #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tobiasgraeber
Copy link

This is more like a workaround for now. Background details:

  • On our system, for a so far unknown reason, the exif data (does not depend on the test image, the exif data is as usuall but its not correctly respected somehow) is somehow in cases of 6 and 8 (see exif orientation screenshots in article of next url) not correctly processed with the core "auto_orient" checkbox option (images are NOT rotated correctly)
  • This commit intoduces a seperate, additional handling, which basically "inverts" the image-orientation in exif data cases 6 and 8.
  • Background info: it should work like this: https://jdhao.github.io/2019/07/31/image_rotation_exif_info/ (and thats also how it is handled so far) but ->

Overall:
It seems somehow not always reliable how to act on the EXIF data. In theory i would totally agree and say that the approach described and explained here (https://jdhao.github.io/2019/07/31/image_rotation_exif_info/) and also currently taken in the core of the extension should be correct, but, under some (sadly not yet absolutely clear) circumstances, the auto-orient feature does not work - at least on our new server. And i could imagine that the same happens for the user (https://github.com/DTM-TYPO3)´s usecase in this issue #60

Also, the problem has been reported and studied already by others like:

....so thats why i introduced in this branch the additional option. Of course, in best case it would not be needed at all but so far at least it provides a workaround for us and maybe its helpfull for others ... :-)

- On our system, for a so far unknown reason, the exif data is somehow in cases of 6 and 8 not correctly processed with the core "auto_orient" checkbox option
- This commit intoduces a seperate, additional handling, which basically "inverts" the image-orientation in exif data cases 6 and 8.
- Background info: it should work like this: https://jdhao.github.io/2019/07/31/image_rotation_exif_info/ (and thats also how it is handled so far) but ->
- that seems somehow not always reliable, see also this study here: https://www.daveperrett.com/articles/2012/07/28/exif-orientation-handling-is-a-ghetto/
- And therefore, this "workaround"-option introduced hereby
Configuration/TCA/Module/Options.php Show resolved Hide resolved
@@ -85,6 +85,12 @@
'type' => 'check',
],
],
'auto_orient_custom' => [
'label' => 'Use custom auto_orient (flop rotate 6 and 8 handling, based on image exif)',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

=> Use a LLL:EXT (XLIFF) label

Furthermore, I'd suggest to add a description field with some further details, such as a (textual) link explaining the problematic with some pieces of software.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, can be done, i leave that up to you :-)

$gifCreator->scalecmd = '-auto-orient ' . $gifCreator->scalecmd;

if ((bool)$ruleset['auto_orient_custom'] === true) {
switch ((int) $orientation) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no blank space before $orientation in CGL

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, can be done, i leave that up to you :-)


if ((bool)$ruleset['auto_orient_custom'] === true) {
switch ((int) $orientation) {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please drop all those useless line breaks between cases, they do not bring anything and are not used either in the rest of the codebase.

@@ -121,7 +127,7 @@
'directories,threshold,file_types,
--palette--;LLL:EXT:frontend/Resources/Private/Language/locallang_ttc.xlf:ALT.imgDimensions;dimensions,
--div--;LLL:EXT:image_autoresize/Resources/Private/Language/locallang_tca.xlf:tabs.options,
auto_orient,keep_metadata,resize_png_with_alpha,conversion_mapping,
auto_orient,auto_orient_custom,keep_metadata,resize_png_with_alpha,conversion_mapping,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is fully related to auto_orient, I'd suggest to group both "auto autorientation" options in a single palette instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants