Skip to content

Fix: Ensure GIF and TIFF are treated as images #12159

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

Merged
merged 6 commits into from
Apr 25, 2023
Merged

Fix: Ensure GIF and TIFF are treated as images #12159

merged 6 commits into from
Apr 25, 2023

Conversation

f-pereira
Copy link
Contributor

Resolved / Related Issues

  • Were these changes approved in an issue or discussion with the project maintainers? In order to prevent extra work, feature requests and changes to the codebase must be approved before the pull request will be reviewed. This prevents extra work for the contributors and maintainers.
    Closes Bug: GIF and TIFF aren't treated as images #12133

Validation
How did you test these changes?

  • Did you build the app and test your changes?
  • Did you check for accessibility? You can use Accessibility Insights for this.
  • Did you remove any strings from the en-us resource file?
    • Did you search the solution to see if the string is still being used?
  • Did you implement any design changes to an existing feature?
    • Was this change approved?
  • Are there any other steps that were used to validate these changes?
    1. Open app
    2. Select a .gif or .tiff file
    3. Should have image options (rotate left, rotate right, etc...)

Screenshots (optional)
Screenshot 2023-04-22 163710

@ferrariofilippo
Copy link
Contributor

There's a bug when rotating GIFs, but I think it's a different issue

@f-pereira
Copy link
Contributor Author

Oh I see, It seems that only the first frame is rotated

@hishitetsu
Copy link
Member

Thank you for your PR! This page will help you to rotate all frames.
https://learn.microsoft.com/en-us/uwp/api/windows.graphics.imaging.bitmapencoder?view=winrt-22621

If you want to encode an additional frame, call GoToNextFrameAsync.

@f-pereira
Copy link
Contributor Author

@hishitetsu Thank you for the documentation! I tried to use it and I found a problem that I am Looking for solution: the first frame rotates well, but if the next frame have a different width/height, it rotates well but It is misplaced (e.g It is placed on top right corner instead of center)

@hishitetsu
Copy link
Member

hishitetsu commented Apr 23, 2023

  • Will there be any problem if all frames had the same width and height?
  • Will rotating four times in a row or rotating left and then right restore all frames to their original state even if there are frames of different widths and heights?

If no to the former and yes to the latter, I think that is still fine.

@f-pereira
Copy link
Contributor Author

@hishitetsu Done!
I tested with a gif where all the frames are equal in size and it worked well.
I also tested with other image formats and it is working

@ferrariofilippo
Copy link
Contributor

@hishitetsu Done! I tested with a gif where all the frames are equal in size and it worked well. I also tested with other image formats and it is working

I'm getting a frame wrong (but if I rotate the gif 4 times everything works). I used this to test

@f-pereira
Copy link
Contributor Author

f-pereira commented Apr 24, 2023

I noticed that it happens when the frames are not equal in size (I checked your gif, the frames size are different)
To confirm this I used this gif

this

Co-authored-by: hishitetsu <66369541+hishitetsu@users.noreply.github.com>
@hishitetsu
Copy link
Member

I noticed that File Explorer does not have the feature to rotate GIFs maybe because the problem above.
This problem should be listed as a separate issue, but it doesn't look like it would be easy to fix and there are GIFs that can be rotated correctly, so I am fine with this.

Copy link
Member

@hishitetsu hishitetsu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ferrariofilippo ferrariofilippo left a comment

Choose a reason for hiding this comment

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

LGTM and works for me!

Copy link
Member

@yaira2 yaira2 left a comment

Choose a reason for hiding this comment

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

Thank you

@yaira2 yaira2 merged commit 5810235 into files-community:main Apr 25, 2023
@yaira2 yaira2 added ready to merge Pull requests that are approved and ready to merge and removed needs - code review labels Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Pull requests that are approved and ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: GIF and TIFF aren't treated as images
4 participants