Skip to content

Optimizing currently included PNGs #199

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 4 commits into from
Aug 16, 2019

Conversation

0xB10C
Copy link
Contributor

@0xB10C 0xB10C commented Aug 14, 2019

Optimized with optipng -o7. Will rebase and add the PNGs from the currently open PRs (e.g. #179, #180, #181) once they are merged (if necessary). Happy to squash before merging this PR.

refs #195

Copy link
Contributor

@bitschmidty bitschmidty left a comment

Choose a reason for hiding this comment

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

@0xB10C Thanks for getting on this. Based on your original suggestion (thanks!) I optimized the compatibility matrix screenshots and logos for both the merged and unmerged compatibility commits. This PR finishes the job.

It looks like there are some images in /img/sponsors/ that were not changed. Were those already optimized?

add the PNGs from the currently open PRs

Can you clarify this for me?

@0xB10C
Copy link
Contributor Author

0xB10C commented Aug 14, 2019

I optimized the compatibility matrix screenshots and logos for both the merged and unmerged compatibility commits. This PR finishes the job.

add the PNGs from the currently open PRs

Can you clarify this for me?

Oh sorry, I did't know you had already optimized the PNGs in the open PRs (i.e. the ones mentioned above). Then all PNGs should be optimized.

It looks like there are some images in /img/sponsors/ that were not changed. Were those already optimized?

The images in img/sponsors/ are JPEGs and not PNGs. Not sure if we can really do lossless optimization on these. But I'll have a look.

@0xB10C 0xB10C mentioned this pull request Aug 14, 2019
2 tasks
@jnewbery
Copy link
Contributor

#200 is merged. Can this be closed?

@0xB10C
Copy link
Contributor Author

0xB10C commented Aug 14, 2019

It looks like there are some images in /img/sponsors/ that were not changed. Were those already optimized?

The images in img/sponsors/ are JPEGs and not PNGs. Not sure if we can really do lossless optimization on these. But I'll have a look.

The three JPEGs in img/sponsors/, dong.jpg and newbery.jpg in img/team/ are already optimized according to the tool jpegoptim with default parameters.

#200 is merged. Can this be closed?

I think this PR should cover all PNGs. I'll mark it as ready for review.

@0xB10C 0xB10C marked this pull request as ready for review August 14, 2019 22:31
@harding
Copy link
Collaborator

harding commented Aug 16, 2019

ACK 773cd55 . Lightly tested by looking at each modified image in my preferred image viewer (geeqie): geeqie $( git diff HEAD^^^^ --name-only ) . I didn't look at the originals for comparison, but each image matched what I'd expect from its file name.

Interestingly, both the original and modified copies of img/posts/rbf-in-the-wild/rbf-green-multiple-bump-options.png gave me a Warning: Failed to decode Exif metadata.. The image decodes fine, otherwise. Out of curiousity, @bitschmidty did you do something different for this image compared to the others you took for that blog post?

@0xB10C
Copy link
Contributor Author

0xB10C commented Aug 16, 2019

Thank you for the review @harding!

@jnewbery jnewbery merged commit 7a2f8fc into bitcoinops:master Aug 16, 2019
@jnewbery
Copy link
Contributor

Thanks @0xB10C and @harding 🖼️

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.

4 participants