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 Pixbuf::put_pixel when alpha channel disabled #60

Merged
merged 2 commits into from Jan 30, 2018

Conversation

Projects
None yet
3 participants
@cai-lw
Contributor

cai-lw commented Jan 29, 2018

#59

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Jan 29, 2018

Good catch!

cc @EPashkin @sdroege

@sdroege

This comment has been minimized.

Member

sdroege commented Jan 29, 2018

Makes sense but will still explode on e.g. grayscale pictures, or not? Those should only have a single channel.

Also accessing the pixel data with that function is super inefficient :) Ideally there would be a way to get a &[u8] (and mut) for the pixel data, and the rowstride/etc. To directly work on the pixel data.

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Jan 29, 2018

Good point, we should only write as many bytes as there are channels.

@cai-lw

This comment has been minimized.

Contributor

cai-lw commented Jan 29, 2018

@sdroege The original documentation (https://developer.gnome.org/gdk-pixbuf/stable/gdk-pixbuf-The-GdkPixbuf-Structure.html#GdkPixbuf--n-channels) says:

Currently, only 3 or 4 samples per pixel are supported.

So there's no need to worry about grayscale pictures.

Pixbuf::get_pixels already exists, which returns the raw data as &mut [u8]. But I am now working on some computer graphics stuff that computes the image pixel by pixel, so I prefer to use put_pixel in this scenario.

@sdroege

This comment has been minimized.

Member

sdroege commented Jan 29, 2018

Maybe an assertion for the number of channels should be added then, to be sure this does not fall apart at a later time.

@sdroege

This comment has been minimized.

Member

sdroege commented Jan 29, 2018

👍

@GuillaumeGomez

This comment has been minimized.

Member

GuillaumeGomez commented Jan 29, 2018

Just waiting for CIs and then I merge. Thanks!

@sdroege

This comment has been minimized.

Member

sdroege commented Jan 30, 2018

CI is happy

@GuillaumeGomez GuillaumeGomez merged commit cbd73f6 into gtk-rs:master Jan 30, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment