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

Improve support for animated image importing #1801

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Improve support for animated image importing #1801

merged 7 commits into from
Dec 12, 2023

Conversation

scribblemaniac
Copy link
Member

@scribblemaniac scribblemaniac commented Dec 4, 2023 •
edited by J5lx
Loading

This PR contains a few loosely related changes to image importing. Specifically:

  • Add support for importing animated WebP images (same Qt code that handles animated GIF import)
  • Progress dialog for animated image import with abort support
  • Separate logic handling single and animated image importing
  • Make movie export modal so the user can't perform other operations that might interfere with the export (report)
  • Fix possible issue with error checking during image importing. I did not try to produce the issue, but based on the code it would probably handle some image corruption errors incorrectly.
  • Add non-animated WebP image import/export

@scribblemaniac
Fix movie export not being a modal operation
5a8fc4e
@scribblemaniac
Make DoubleProgressDialog inherit from QProgressDialog
f41aa62 
Probably not done initially because it adds a lot of member functions
and properties that are not used by the double progress dialog.
However it handles canceling much better than we do manually,
including canceling with keyboard shortcuts or by closing the dialog.
@scribblemaniac
Improve gif import progress dialog and refactor
9db12b7 
The progress dialog now updates as it imports the gif, and can be
aborted during the import. Some refactoring was done to move some
logic around.
@scribblemaniac
Remove animated image handling from Editor::importBitmapImage
12a2084 
The error checking has also been modified to hopefully be more
robust as it no longer depends on reader.size() being invalid,
which might not occur during certain types of image corruption,
and may occur without corruption for certain types of image formats.
@scribblemaniac
Add support for WebP import through a generic "Animated Image" import
f965a9f
@scribblemaniac
Add non-animated WEBP import/export
b632734
MrStevns
MrStevns requested changes Dec 10, 2023
View reviewed changes
Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

Overall i like the changes, there's just one thing I bumped into that should be fixed.

The title for the import dialog is now saying "animated image" when it should be sequence and vise versa.

app/src/doubleprogressdialog.h Outdated Show resolved Hide resolved
app/src/importimageseqdialog.cpp Outdated Show resolved Hide resolved
J5lx
J5lx reviewed Dec 10, 2023
View reviewed changes
app/src/actioncommands.cpp Outdated Show resolved Hide resolved
MrStevns
MrStevns approved these changes Dec 12, 2023
View reviewed changes
Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MrStevns MrStevns merged commit 3c9a7a4 into pencil2d:master Dec 12, 2023
8 checks passed
@scribblemaniac scribblemaniac deleted the anim-img-improvements branch December 12, 2023 06:43
@J5lx J5lx added this to the v0.6.7 milestone Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrStevns MrStevns MrStevns approved these changes

@J5lx J5lx Awaiting requested review from J5lx

Assignees
No one assigned
Labels
Bitmap Enhancement File Export File Import
Projects
Enhancement Priority
Status: No status
Pull Request Priority
Status: Merged
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@scribblemaniac @MrStevns @J5lx