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.

Sign up for GitHub

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

Jump to bottom

imaginary - fix autorotate for heic and improve the logic #37140

Merged
merged 1 commit into from
Mar 9, 2023
Merged

imaginary - fix autorotate for heic and improve the logic #37140

merged 1 commit into from
Mar 9, 2023

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Mar 8, 2023

Fix #35643

@szaimen szaimen added bug 2. developing Work in progress labels Mar 8, 2023
@szaimen szaimen added this to the Nextcloud 27 milestone Mar 8, 2023
@szaimen szaimen force-pushed the enh/35643/fix-heic-previews branch 6 times, most recently from 7cf4ad1 to 2a538b1 Compare March 8, 2023 22:42
@szaimen
imaginary - fix autorotate and improve the logic
b39329c 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen szaimen force-pushed the enh/35643/fix-heic-previews branch from 2a538b1 to b39329c Compare March 8, 2023 23:11
@szaimen szaimen marked this pull request as ready for review March 8, 2023 23:18
@szaimen szaimen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 8, 2023
@szaimen
Copy link
Contributor Author

szaimen commented Mar 8, 2023

/backport to stable26

@szaimen
Copy link
Contributor Author

szaimen commented Mar 8, 2023

/backport to stable25

@szaimen szaimen requested review from PVince81, st3iny, come-nc, a team, ArtificialOwl and icewind1991 and removed request for PVince81 and a team March 8, 2023 23:18
@szaimen szaimen changed the title imaginary - fix autorotate and improve the logic imaginary - fix autorotate for heic and improve the logic Mar 9, 2023
artonge
artonge approved these changes Mar 9, 2023
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Fine beside question

lib/private/Preview/Imaginary.php
'quality' => $quality,
]
]
} elseif ($autorotate) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are $autorotate and $convert mutually exclusive ? If no, I would convert to if instead of ifesle

Copy link
Contributor Author

@szaimen szaimen Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Currently they are mutually exclusive as autorate only makes sense for images (and not other files) that dont need to be converted and the other way around.

@szaimen szaimen merged commit 691aa8d into master Mar 9, 2023
@szaimen szaimen deleted the enh/35643/fix-heic-previews branch March 9, 2023 09:25
@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable25

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable26

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable25

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable26

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable25 failed. Please do this backport manually.

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable25

@szaimen
Copy link
Contributor Author

szaimen commented Mar 9, 2023

/backport to stable26

This was referenced Mar 9, 2023
Merged
Merged
@loeffelpan
Copy link

Thanks for backporting this.
But is it really fixed as autorotation is just disables for HEIC?
Any thoughts how make autorotation work for HEIC?

@matiasdelellis matiasdelellis mentioned this pull request May 21, 2023
Closed
@joshtrichards joshtrichards mentioned this pull request Nov 10, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@artonge artonge artonge approved these changes

@st3iny st3iny Awaiting requested review from st3iny

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@icewind1991 icewind1991 Awaiting requested review from icewind1991 icewind1991 was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
Nextcloud 27
Development

Successfully merging this pull request may close these issues.

Imaginary preview generation is not working as expected for folders containing HEIC images.
4 participants
@szaimen @loeffelpan @artonge @come-nc