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

Grayscale PNGs are rendered incorrectly #3154

Closed
narfg opened this issue Aug 26, 2014 · 2 comments
Closed

Grayscale PNGs are rendered incorrectly #3154

narfg opened this issue Aug 26, 2014 · 2 comments
Assignees

Comments

@narfg
Copy link

@narfg narfg commented Aug 26, 2014

Servo renders grayscale PNGs incorrectly.
Both images with and without alpha channel are broken.
Test e.g. with the German wikipedia logo:
./servo http://upload.wikimedia.org/wikipedia/commons/e/ec/Wikipedia-logo-v2-de.png

screenshot

The problem doesn't occur with RGB images.
I experience this problem with CPU and GPU rendering and the skia backend.

I tried to find the source of this problem and found these locations which may be helpful for someone trying to fix the bug.

The bug may be in src/components/gfx/render_context.rs draw_image
let azure_surface = draw_target_ref.create_source_surface_from_data(image.pixels.as_slice(), size, stride as i32, B8G8R8A8);
where it might be possible to create a grayscale surface instead of B8G8R8A8 (I don't know if this is supported).

Another possible location may be

src/support/azure/rust-azure/src/gfx/2d/SourceSurfaceSkia.cpp
SourceSurfaceSkia::InitFromData

where it seems to be assumed that the input image and the surface have the same color format.

Of course there are other ways to fix this, like converting the image to RGB in the decoding step but this may be wasteful in terms of memory.
Maybe someone with experience in Azure can take a look at this or file a bug report in Azure if this is not servo's fault.

SimonSapin added a commit to servo/rust-png that referenced this issue Aug 26, 2014
… by making pixels fields of the enum variants.

This should help prevent bugs such as servo/servo#3154
@SimonSapin
Copy link
Member

@SimonSapin SimonSapin commented Aug 26, 2014

So the problem apparently is that we’re accessing the pixels without checking the image’s color type. Forcing us to do so would help prevent this kind of bug. Enums are designed to do just that: servo/rust-png#42

SimonSapin added a commit to servo/rust-png that referenced this issue Aug 26, 2014
… by making pixels fields of the enum variants.

This should help prevent bugs such as servo/servo#3154
SimonSapin added a commit to SimonSapin/servo that referenced this issue Aug 26, 2014
servo/rust-png#42

This does *not* fix the issue in servo#3154, but makes it fail!()
instead of (maybe?) reading uninitalized memory.
mbrubeck added a commit to mbrubeck/rust-png that referenced this issue Sep 3, 2014
mbrubeck added a commit to mbrubeck/rust-png that referenced this issue Sep 3, 2014
mbrubeck added a commit to mbrubeck/rust-png that referenced this issue Sep 3, 2014
mbrubeck added a commit to mbrubeck/rust-png that referenced this issue Sep 3, 2014
@mbrubeck mbrubeck self-assigned this Sep 3, 2014
@mbrubeck mbrubeck added the I-wrong label Sep 3, 2014
@mbrubeck
Copy link
Contributor

@mbrubeck mbrubeck commented Sep 10, 2014

Fixed by servo/rust-png#45.

@mbrubeck mbrubeck closed this Sep 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.