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

Copy image as PNG file on Windows #141

Merged
merged 2 commits into from
Apr 28, 2024

Conversation

Klavionik
Copy link
Contributor

@complexspaces Hello! I decided to just go ahead and open a PR regarding the issue #135. Hope you don't mind.

I'm pretty new to Rust, so pardon me if I did something plain wrong here. :) Just wanted to help you understand the idea that I proposed in more detail.

Here's how it looks if you paste a transparent image into PowerPoint now:

transparent

And this is how it looked before:

opaque

@Klavionik Klavionik force-pushed the copy-image-as-png-file branch from 3a2b851 to 67e0b06 Compare March 5, 2024 07:57
@Klavionik
Copy link
Contributor Author

@complexspaces Just a kind reminder! I would be glad if you could find some time to review this.

@complexspaces complexspaces force-pushed the copy-image-as-png-file branch from 67e0b06 to 9f39de1 Compare April 28, 2024 05:51
Copy link
Collaborator

@complexspaces complexspaces 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 for taking the time to make a PR! I took a bit too long to finish looking over this one, but I wanted to better understand how more of the Windows ecosystem handles transparent images. After looking through Firefox, Java, Chromium, etc I've convinced myself this is the right approach for better compatibility.

One thing I noticed is that other apps use DIBV5 plus BI_RGB as a secondary option to PNG encoding data. This is something arboard can look at later but isn't a blocker for this PR.

I took a look through your changes and only had a few minor codestyle comments. This needed rebased on main as well to pick up the recent image 0.25 update too. In order to save some time, since I already took a while to get around to reviewing this, I updated the branch myself with my suggestions.

@complexspaces complexspaces force-pushed the copy-image-as-png-file branch from 9f39de1 to 3654907 Compare April 28, 2024 06:53
@complexspaces complexspaces merged commit 83740b7 into 1Password:master Apr 28, 2024
11 checks passed
@Klavionik
Copy link
Contributor Author

Glad to hear it's merged! Thank you too, for taking care of the PR.

@complexspaces
Copy link
Collaborator

This is now released in version 3.4.0 on crates.io. Please let us know if you run into any more issues.

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