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, #6928] bug/transparent-png-does-not-work-#6928 #6947

Conversation

ben-biddington
Copy link
Contributor

@ben-biddington ben-biddington commented Jul 25, 2024

Contributor checklist:

  • My contribution is not related to translations.
  • My commits are in nice logical chunks with good commit messages
  • My changes are rebased on the latest main branch
  • A npm run ready run passes successfully (more about tests here)
  • My changes are ready to be shipped to users

Run these new tests with:

npm run generate && npm run test-electron -- --grep="image processing"

Fix

All I have done is modify ts/util/scaleImageToLevel.ts to preserve file type instead of hard-wiring to jpeg and the test passes.

Not sure of other implications or possible reasons for changing png to jpg that I may have missed.

Before

Transparent pngs are re-rencoded as jpg and their backgrounds are black.

image

After

image

[6881] File size increase when sending images

While doing that I wondered about #6881 as well so added a characterizsation test for that which demonstrates the reported behaviour.

Not a fix just something to make pass.

@ben-biddington ben-biddington changed the title [#6928, REPRODUCTION] bug/transparent-png-does-not-work-#6928 [REPRODUCTION, #6928] bug/transparent-png-does-not-work-#6928 Jul 25, 2024
@ben-biddington ben-biddington force-pushed the bug/transparent-png-does-not-work-#6928 branch from c4e540d to c43cc86 Compare July 29, 2024 01:57
@ben-biddington ben-biddington changed the title [REPRODUCTION, #6928] bug/transparent-png-does-not-work-#6928 [FIX, #6928] bug/transparent-png-does-not-work-#6928 Jul 29, 2024
@ben-biddington ben-biddington force-pushed the bug/transparent-png-does-not-work-#6928 branch from f836da2 to 64440e7 Compare July 29, 2024 02:23
[signalapp#6928] Transparent Png Doesn't Work (Even With Files)

Add a failing test to show that in some cases uploading a png file results in a jpg being
produced.

[signalapp#6881] File size increase when sending images

Add a passing test that automates the example written in the issue,
where a file's size increases after uploading.

Run with:

  npm run generate && npm run test-electron -- --grep="image processing"

It does add an extra devDependency, hope that's okay. It's for
inspecting jpeg images.
This does make the test pass, not sure if there are wider implications.

Is there a reason png files need to be converted to jpeg sometimes?
@ben-biddington ben-biddington force-pushed the bug/transparent-png-does-not-work-#6928 branch from dd1805f to 562d35e Compare July 29, 2024 23:23
Copy link

stale bot commented Oct 28, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 28, 2024
Copy link

stale bot commented Nov 9, 2024

This issue has been closed due to inactivity.

@stale stale bot closed this Nov 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

1 participant