signalapp / Signal-Desktop

A private messenger for Windows, macOS, and Linux.
https://signal.org/download
GNU Affero General Public License v3.0
14.6k stars 2.66k forks source link

Copying image from lightbox alters its color #5778

Open awaitlink opened 2 years ago

awaitlink commented 2 years ago

Bug Description

If the image is copied via right-clicking from the lightbox, it noticeably changes color.

Saving the image via the "save" icon in the lightbox instead of step 4 doesn't appear to exhibit this behavior.

I have Night Shift enabled, but the behavior seems the same if I manually disable it.

Steps to Reproduce

Note: these steps include sending the image, but the bug seems to happen at the moment of copying the image. Once copied, the image can be pasted in other software and the same results can be observed.

  1. Paste an image into the composer.
  2. Send the message/image.
  3. Open (the lightbox for) the image from the message that was just sent.
  4. Right-click to copy the image.
  5. Repeat steps 1-4 a few times.

Actual Result: Image changes color. Expected Result: Image appears (visually) unmodified throughout this process.

Screenshots

These are the images from the messages in Signal Desktop, saved using the aforementioned "save" icon:

"Original" After 1 repetition of steps 1-4 After 2 repetitions After 3 repetitions After 4 repetitions

Platform Info

Signal Version: 5.31.0-beta.1 Operating System: macOS 12.2 Linked Device Version: not applicable

Link to Debug Log

https://debuglogs.org/2f1a6e80f4577abf4192b8dd1bc84fe5959e87d62b325e4bad8a0723d987b959.gz

josh-signal commented 2 years ago

Thank you for your detailed report, however I can't quite repro this but I'll continue trying

Screen Shot 2022-02-03 at 12 36 05 PM
scottnonnenberg-signal commented 2 years ago

@u32i64 Can you tell us about your macOS graphics hardware/drivers and display settings?

awaitlink commented 2 years ago

Thank you for looking into this! The graphics hardware is stock on a Mac mini (Late 2014) (1.4GHz). The U28E590 display is connected via HDMI.

Here's what's displayed in macOS' System Information in Graphics/Displays:

Intel HD Graphics 5000:
  Chipset Model:    Intel HD Graphics 5000
  Type:         GPU
  Bus:          Built-In
  VRAM (Dynamic, Max):  1536 MB
  Vendor:       Intel
  Device ID:        0x0a26
  Revision ID:      0x0009
  Metal Family:     Supported, Metal GPUFamily macOS 1
  Displays:
    U28E590:
      Resolution:           3840 x 2160 (2160p/4K UHD 1 - Ultra High Definition)
      UI Looks like:            1920 x 1080 @ 30.00Hz
      Framebuffer Depth:        24-Bit Color (ARGB8888)
      Display Serial Number:        [REDACTED]  
      Main Display:         Yes
      Mirror:               Off
      Online:               Yes
      Rotation:             Supported
      Automatically Adjust Brightness:  Yes

I haven't installed any custom graphics drivers. This seems to be the relevant driver in Extensions:

AppleIntelHD5000Graphics:
  Version:      18.4.6
  Last Modified:    22.01.2022, 10:42
  Bundle ID:        com.apple.driver.AppleIntelHD5000Graphics
  Notarized:        Yes
  Loaded:       Yes
  Get Info String:  AppleIntelHD5000Graphics 18.4.6 10
  Obtained from:    Apple
  Kind:         Intel
  Architectures:    x86_64
  64-Bit (Intel):   Yes
  Location:     /System/Library/Extensions/AppleIntelHD5000Graphics.kext
  Kext Version:     18.0.4
  Load Address:     18446743522213360000
  Loadable:     Yes
  Dependencies:     Satisfied
  Signed by:        Software Signing, Apple Code Signing Certification Authority, Apple Root CA

This is the chosen color profile in System Preferences (though I've just tried SD 170M-A and Display as well and they seem to produce a similar/same result):

Let me know if you need any further information.

Shnub commented 2 years ago

I can reproduce this on macOS 10.14.6 with Signal Desktop 5.31.0.

(tl;dr: I suspect Signal doesn't handle raw image data pasted from the clipboard properly (as opposed to image files pasted from the clipboard). The copy action from the lightbox may contribute to the effect, but may not be the root cause.)

I used the following test graphic to track what's happening with the colors here:

rgbymcbgw

This has patches of the following RGB values:

255, 0, 0 (red) 0, 255, 0 (green) 0, 0, 255 (blue)
255, 255, 0 (yellow) 255, 0, 255 (magenta) 0, 255, 255 (cyan)
0, 0, 0 (black) 128, 128, 128 (middle gray) 255, 255, 255 (white)

The file is in the sRGB color space, so even though Signal removes the EXIF color space tag from the file, the image's color space shouldn't be an issue here since Signal tags the file it actually sends with sRGB again.

Here are the files saved out of Signal after performing the steps @u32i64 described:

Original image sent through Signal: 0

1st iteration of copy last sent image from the lightbox, paste in composer and send again: 1

2nd iteration: 2

3rd iteration: 3

10th iteration: 10

I included the original sent image to confirm colors are not altered at this step already. I included the 10th iteration because the changes after a few repetitions may be very hard to see, depending on your monitor's quality and setup.

The RGB values change in the following progression (changes in bold):

Original image sent through Signal:
255, 0, 0 0, 255, 0 0, 0, 255
255, 255, 0 255, 0, 255 0, 255, 255
0, 0, 0 128, 128, 128 255, 255, 255
1st iteration:
255, 0, 0 0, 255, 0 27, 0, 255
254, 255, 0 255, 0, 255 0, 255, 255
0, 0, 0 129, 129, 129 255, 255, 255
2nd iteration:
255, 0, 0 0, 255, 0 42, 0, 255
253, 255, 0 255, 0, 255 0, 255, 255
0, 0, 0 130, 130, 130 255, 255, 255
3rd iteration:
255, 0, 0 0, 255, 0 55, 0, 255
251, 255, 0 255, 0, 255 0, 255, 255
0, 0, 0 131, 131, 131 255, 255, 255
10th iteration:
255, 0, 0 0, 255, 0 155, 0, 255
220, 255, 0 255, 0, 255 0, 255, 255
0, 0, 0 138, 138, 138 255, 255, 255

So blue gets a heavy dose of red added with each iteration, resulting in it having shifted to purple by the 10th; yellow gets red subtracted; and middle grey gets lighter.

It doesn't matter for the issue at hand whether the original file is pasted into Signal als PNG or JPEG, and also not whether pasted images are sent from Signal with the image quality selector at "Standard" or "High". Lastly, the lightbox display of the image itself seems fine, at least on my machine, as confirmed by copying from there, pasting directly into an image editor and checking the colors. (I'm not sure I understood correctly, but apparently @u32i64 did see changes in that case...?) [Edit: I was mistaken, see my next comment.]

Anyway, my suspicion is that the issue must be an interaction between the clipboard and Signal's handling of image data from it. If I was to venture a guess, it would be that Signal has an issue with handling raw image data from the clipboard (as opposed to whole image files being pasted from the clipboard). FWIW, if I copy my test image from above as raw image data directly out of Affinity Photo, I also see subtle changes after pasting and sending (and then saving the sent file) in Signal, though they are not iterative and not as dramatic as what happens when copying from the lightbox, so the latter may still play a role.

If my suspicion is correct, an easy workaround for this particular issue would be to make the lightbox copy the actual image file instead of raw image data. But this would not solve the issue of Signal handling pasted image data improperly (if that's what's actually happening—as I said it's just an educated guess, as I don't know enough about how the clipboard actually works). [Edit: I was mistaken, see my next comment.]

@josh-signal I can't detect any change in color in your file either (even inspecting the RGB values in an image editor), but try using my test file above—this may yield different results than screenshotting the Signal app and using that screenshot as a test file. (Screenshots use your system's display profile, which is more or less unique and thus an unwanted variable here.) Also, I'd recommend saving the files from Signal and sharing those instead of making screenshots of the results.

@u32i64 Take care not to use SD 170M-A as your display profile, as it's not a display profile at all—it's an old color space used for television. Make sure that in the Color tab in System Preferences > Display > Color, "Show profiles for this display only" is checked, which will get rid of anything that is not explicitly a profile for your particular monitor; then select a profile that remains, in your case probably "U28E590", if that's the Samsung monitor model you're using, otherwise probably "Display".

awaitlink commented 2 years ago

Lastly, the lightbox display of the image itself seems fine, at least on my machine, as confirmed by copying from there, pasting directly into an image editor and checking the colors. (I'm not sure I understood correctly, but apparently @u32i64 did see changes in that case...?)

On macOS 12.2.1 with Signal 5.32.0-beta.1 I now tried the following:

  1. Right-clicked the very first image in your post in Safari and chose Save Linked File to "Downloads".
  2. Dragged the image from the Downloads popup in Dock to Signal.
  3. Sent the message.
  4. Opened the lightbox for that message, right-clicked and copied the image.
  5. Launched macOS' Preview.app.
  6. Pressed ⌘+N to create a document from the clipboard.
  7. Saved the resulting document.

This is the result (the PNG image saved in step 7 that I dragged from Downloads in the Dock into GitHub in Safari) (image 1):

This is the image saved from the lightbox using the save icon there, then dragged here in the same way (image 2):

I'm not exactly sure what would be a reliable way/editor to get the color values the same way as you did, so I just used the color picker from macOS' Color window. It grabs from the screen which I understand might not be precise, nevertheless I observed the following when picking the top-right square:

I'll try doing step 4 now and pasting the image directly into Safari here (image 3):

On image 3 the top-right square appears to be rgb(55, 0, 255) on my screen.

These results seem to indicate that the copying is where the problem occurs. Although, I suppose, it could be that Signal, Preview, and Safari share the same issue with pasting the image, or I guess it could be macOS itself (perhaps the image editor you use handles something in a manual way, different from what macOS provides?).

@u32i64 Take care not to use SD 170M-A as your display profile, as it's not a display profile at all—it's an old color space used for television. Make sure that in the Color tab in System Preferences > Display > Color, "Show profiles for this display only" is checked, which will get rid of anything that is not explicitly a profile for your particular monitor; then select a profile that remains, in your case probably "U28E590", if that's the Samsung monitor model you're using, otherwise probably "Display".

Thanks for the advice! I don't see that checkbox (perhaps because the Displays preferences pane was recently redesigned in macOS), however the menu that appears after pressing Customize in that picker has the section "Color profiles for this display", which appears to be the same as the section above the divider of the picker (see my screenshot above). I have chosen U28E590 in that first section. Strangely, SD 170M-A is also in that section.

Shnub commented 2 years ago

Ah, I think I see what the issue is. The image data copied from Signal's lightbox is being improperly tagged with the system's display profile. That's why I wasn't seeing any change in RGB values after copying while you did—your monitor apparently has a smaller color gamut than sRGB, which means colors will get desaturated when the image data is assigned the smaller-than-sRGB monitor profile, while my monitor has a larger gamut, hence RGB values would be retained but colors would look oversaturated (unless sRGB was manually re-assigned to the image).

That is definitely a bug in Signal. The image file produced and sent by Signal is tagged with sRGB, so if that image is viewed in the lightbox and copied, the image data (in whatever form it is actually copied, I still don't know) should be tagged with sRGB as well, not with the system's display profile.

I could also verify there is no issue with the clipboard and Signal's composer, contrary to my previous suspicion. To see this, one simply has to assign sRGB (or "sRGB IEC61966-2.1" as the profile is called in full) as one's display profile, restart Signal (apparently it polls the display profile only once at startup), then go through the procedure described in the original bug report—no change in colors should occur even with a large number of repetitions, RGB values should stay exactly the same.

(Make sure to switch back your monitor profile to the proper one afterwards. sRGB is also not a display profile and will yield terrible colors unless your monitor's gamut is exatly sRGB.)

Strangely, SD 170M-A is also in that section.

I've seen that before with some monitors. Perhaps the monitor identifies itself as an SD-170M-adhering device (like many TVs would), causing macOS to think it's a fitting profile. I'd still ignore it (or disable it, if possible) and use the one that is named after your monitor's model.

stale[bot] commented 2 years ago

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.

awaitlink commented 2 years ago

I can still reproduce the issue, with a different setup: Huawei MateView connected to a Mac mini (M1, 2020) via USB-C (although the colors change differently than on the previous setup).

It's reproducible regardless of whether the monitor's "color gamut" setting is set to sRGB, DCI-P3, or Native (even with restarting Signal after changing the setting). However, if I set the display profile in macOS System Preferences to sRGB IEC61966-2.1 as per the comment above instead of MateView and restart Signal, then there is no visual change in colors.

https://debuglogs.org/desktop/5.44.0-beta.1/4bd273c6c55f511428102c6d48cebdf4f5a845d42be12f0d5f57db334e0c5fae.gz

stale[bot] commented 2 years ago

This issue has been closed due to inactivity.

awaitlink commented 2 years ago

@scottnonnenberg-signal Sorry for the ping, but the stale bot closed this despite the fact that there was activity.

It seems the underlying issue has been found by @Shnub in https://github.com/signalapp/Signal-Desktop/issues/5778#issuecomment-1036596846.

Although if there's more information needed to debug this, please let me know 🙂.

stale[bot] commented 2 years ago

This issue has been closed due to inactivity.