Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(9)

Issue 1226933005: DM: swizzle BGRA to RGBA before calculating pixel MD5. (Closed)

Created:
5 years, 5 months ago by mtklein_C
Modified:
5 years, 5 months ago
Reviewers:
mtklein, reed1
CC:
reviews_skia.org, stephana, jcgregorio
Base URL:
https://skia.googlesource.com/skia.git@master
Target Ref:
refs/heads/master
Project:
skia
Visibility:
Public.

Description

DM: swizzle BGRA to RGBA before calculating pixel MD5. We name our .pngs by pixel hashes for gold. For 8888 images, we're hashing SkPMColors, which have platform-dependent order: BGRA on Linux and Windows, RGBA otherwise. This means we can end up with pixel-identical pngs with different hashes, which is confusing. This CL standardizes on RGBA for 8888 configs, arbitrarily chosen so that Android ends up a no-op. Long-term, this should eliminate most of the 0-pixel-diff problems we see on gold.skia.org. There are other ways to end up with the same .png from different SkBitmaps (think, red 565 square vs. red 8888 square) but they're rather less common / likely. This will temporarily create a giant 0-pixel-diff problem on gold.skia.org. Any Linux or Windows images which are not already pixel-identical to a Mac or Android image should show up as untriaged hashes that are pixel-identical to their version just before landing (we're only changing the hash, not the .png). This means anything vaguely platform dependent (fonts, GPUs) will probably show up as needing a triage but with a zero diff from a previous image. If this goes well, we might do the same for 565. Just want to leave them out for now to cut down on the triaging I need to do in one go. BUG=skia: Committed: https://skia.googlesource.com/skia/+/38408460addd4104f4b321bd085f6855c902231b

Patch Set 1 #

Patch Set 2 : 565 note #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -1 line) Patch
M dm/DM.cpp View 1 1 chunk +11 lines, -1 line 0 comments Download

Messages

Total messages: 8 (3 generated)
mtklein_C
5 years, 5 months ago (2015-07-07 22:05:12 UTC) #2
mtklein
+CC Joe: we think we finally figured out the mystery of 0-pixel-diffs.
5 years, 5 months ago (2015-07-07 22:08:08 UTC) #4
reed1
lgtm
5 years, 5 months ago (2015-07-08 14:15:32 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1226933005/20001
5 years, 5 months ago (2015-07-08 14:17:43 UTC) #7
commit-bot: I haz the power
5 years, 5 months ago (2015-07-08 14:25:33 UTC) #8
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/38408460addd4104f4b321bd085f6855c902231b

Powered by Google App Engine
This is Rietveld 408576698