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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant