Created attachment 73520 [details] simpler png loop Hi, the loop looks simpler this way. I may have miss something here, though. Pascal
Comment on attachment 73520 [details] simpler png loop This isn't quite right. If row 0 has alpha and row 1 does not, you'll setHasAlpha(true) on the first row and (false) on the second. Instead you should probably do: if (nonTrivialAlpha) buffer.setHasAlpha(true); ...at the end. It would be good to add a pixel test with a png that exercises this particular case.
Created attachment 73538 [details] updated patch Fixed the logic, and made sure buffer.setHasAlpha() was called once, like before. Added a pixel test for this case.
Comment on attachment 73538 [details] updated patch This patch is incorrectly based. Please use "webkit-patch upload" or svn-create-patch to create your patches so they can be processed by all our automated tools.
Comment on attachment 73538 [details] updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=73538&action=review Does the test image render visibly wrong with your original patch? If so, I'm basically OK with this patch (can't r+, not a reviewer). > third_party/WebKit/WebCore/ChangeLog:9 > + Tests should stay be 1:1 the same. > + Added a special test for this case: png_per_row_alpha_decoding.html Just say: "Test: fast/images/png_per_row_alpha_decoding.html" -- it's implied that image output won't change.
Created attachment 73539 [details] attachment with correct path base same as 73538, but with correct path
Comment on attachment 73539 [details] attachment with correct path base Marking r? for a few seconds so the bots pick up this change for analysis.
Comment on attachment 73539 [details] attachment with correct path base If Peter's happy, this looks good to me. Please address Peter's nit with the ChangeLog before landing.
Created attachment 73548 [details] updated patch updated patch, with smaller PNG test image and fixed ChangeLog. This test-PNG has an alpha-less first and last row, whereas the center rows do have alpha. So whether setHasAlpha() occurs first or last, it would be forcing a no-alpha bad image. That being said, i see no effect of setHasAlpha() on linux. The renderer seems to ignore this global flag and use whatever alpha value is associated with each pixels.
Comment on attachment 73548 [details] updated patch Rejecting patch 73548 from commit-queue. pascal.massimino@gmail.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in WebKitTools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights.
Comment on attachment 73548 [details] updated patch Clearing flags on attachment: 73548 Committed r71784: <http://trac.webkit.org/changeset/71784>
All reviewed patches have been landed. Closing bug.
BTW, this was missing the .checksum file, I added it in http://trac.webkit.org/changeset/71836.