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/minor export issues #1123

Merged
merged 11 commits into from Dec 4, 2018

Conversation

Projects
None yet
4 participants
@przet
Copy link
Contributor

przet commented Nov 3, 2018

Additional detail/discussion: #1090

Tested on Linux and Windows. All tests pass.

Problems with the Image Exporter (single frame):

  • Transparency can be checked off even for image formats that do not support transparency. The checkbox should instead be disabled when jpg is selected.
  • Bmp export with transparency is broken. I think this is a Qt issue, but I can't get bmps to save properly with transparency. Theoretically the file format supports it, so perhaps experiment with different QImage formats and search around the docs for some more information. Failing that, disable transparency for bmp as well.

Problems with the Image Sequence Exporter (multi frame):

  • Again the transparency checkbox should be disabled for jpg.
  • Bmp has the same issue here, so the same solution should be used here as with the Image Exporter.
  • When changing the image file format, the filename changes incorrectly. It goes from frame.test..png to frame.jpg for example, removing everything after the first period when it should be removing everything after the last period to strip the extension.
  • For some reason frame.bmp exports as frame.bmp0001... but frame.jpg exports as frame0001.jpg. The numbering behavior should be consistent for all formats.

przet added some commits Oct 26, 2018

Transparency check box disappears for JPG.
Initial commit, with logic in the formatChanged "slot", just to check
that this is indeed the right place for it.
Wrote function to hold transparency option logic.
The function is private in the exportimagedialog class.
Fixed a comment.
Was meant to be OR (||) not AND (&&)
Disabled Transparency export for BMP.
After a look around at exporting BMP with transparency, decided
to remove the option for the moment, with the view of opening another
issue with just this, if the maintainers are happy with that.
Fixed error of ignoring . in filename.
If you want to name something frame.test.extension, this now works with
only extension changing now.
Removed comments.
These were comments made for development
Show resolved Hide resolved app/src/exportimagedialog.cpp Outdated
Show resolved Hide resolved app/src/exportimagedialog.cpp Outdated
Show resolved Hide resolved app/src/exportimagedialog.h Outdated
@CandyFace
Copy link
Member

CandyFace left a comment

Looks good przet.
There's however a typo or spelling mistake that I'd like to see corrected before merging.

CandyFace and others added some commits Nov 3, 2018

Typo fix.
Co-Authored-By: przet <przet@tpg.com.au>
Typo fix.
Co-Authored-By: przet <przet@tpg.com.au>
Typo fix.
Co-Authored-By: przet <przet@tpg.com.au>
@przet

This comment has been minimized.

Copy link
Contributor Author

przet commented Nov 3, 2018

Thanks @CandyFace . I have applied the typo fixes.

@scribblemaniac
Copy link
Member

scribblemaniac left a comment

Looks good! All the mentioned issued appear to be fixed.

@chchwy chchwy added this to the 0.6.3 milestone Dec 4, 2018

@chchwy

This comment has been minimized.

Copy link
Member

chchwy commented Dec 4, 2018

Thank you for the contribution @przet and all the reviewers

@chchwy chchwy merged commit 57243ba into pencil2d:master Dec 4, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.