Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1711143002:
DM: support --config {f16,srgb} (Closed)
DescriptionDM: support --config {f16,srgb}
Will need to follow up with enabling f16 and srgb configs.
They're still assert-y.
The GMs 'gamma' and 'gradients' look reassuringly good on f16 and srgb backends.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1711143002
Committed: https://skia.googlesource.com/skia/+/27c3fddaac722086b80a7725f42c14fb694ec4a6
Patch Set 1 #Patch Set 2 : cause a diff #Patch Set 3 : fix endianness #Patch Set 4 : rebase #Patch Set 5 : round and unpremul #Patch Set 6 : turn on f16 config, with a few tests disabled #Patch Set 7 : revert #Patch Set 8 : rebase #Patch Set 9 : skip image -> f16 #Patch Set 10 : dont run #Patch Set 11 : add --config srgb #Patch Set 12 : notes #Patch Set 13 : linearize sRGB #Patch Set 14 : add a grey gm test pattern #Patch Set 15 : win #Patch Set 16 : f16+srgb, no 16-bit pngs #Patch Set 17 : tweaks for clarity #Patch Set 18 : more comments #Patch Set 19 : stray #
Messages
Total messages: 65 (34 generated)
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: ========== to ========== DM: support --config f16, create 16-bit PNGs. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/1
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Some --config f16 GMs seem to work, some assert, some crash without asserting. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Some --config f16 GMs seem to work, some assert, some crash without asserting. Better make sure Gold works with these PNGs before landing. COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Some --config f16 GMs seem to work, some assert, some crash without asserting. Need to: - make sure Gold handles these - round - unpremul COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Some --config f16 GMs seem to work, some assert, some crash without asserting. Need to: - make sure Gold handles these - round - unpremul COMMIT=false BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Looks like we need a fallbacks shim for colorfilters, and lightingshader runs over SkSmallAllocator's budget now. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Mac-Clang-Arm7-Debug-iOS-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-Arm7...) Build-Ubuntu-GCC-Arm64-Debug-Android-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...) Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...) skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED, http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/140001
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-Shared-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow. Looks like we need a fallbacks shim for colorfilters, and lightingshader runs over SkSmallAllocator's budget now. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow, as will enabling f16 or srgb on the bots. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
mtklein@chromium.org changed reviewers: + reed@google.com, stephana@google.com - mtklein@google.com
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow, as will enabling f16 or srgb on the bots. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow, as will enabling f16 or srgb on the bots. When this lands, all new .pngs uploaded to Gold will have 16-bit channels. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/02/24 15:19:07, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Just to clarify from a conversation yesterday. When -f16 is enabled all PNGs are 16-bit while internal buffers (and their hashes) will continue to be 8-bit and match the hashes we already have. --config srgb will cause DM to use 16-bit pixel buffers internally and result in a new config, like 565, 8888 ?
On 2016/02/24 15:31:55, stephana wrote: > On 2016/02/24 15:19:07, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > Just to clarify from a conversation yesterday. > > When -f16 is enabled all PNGs are 16-bit while internal buffers (and their > hashes) will continue to be 8-bit and match the hashes we already have. > --config srgb will cause DM to use 16-bit pixel buffers internally and result in > a new config, like 565, 8888 ? No, --config f16 --config srgb are both new configs like 565 and 8888. f16 is natively 16 bits per component, and srgb is 8 bits per component. I'd have added srgb here but I couldn't quite figure it out yet. When this CL lands, no matter whether the config is natively 8 or 16 bit, it will be uploaded as a 16-bit PNG. The hashes are always computed on the native buffer, so the hashes of existing configs don't change.
looks woot to me
Description was changed from ========== DM: support --config f16, create 16-bit PNGs. --config srgb to follow, as will enabling f16 or srgb on the bots. When this lands, all new .pngs uploaded to Gold will have 16-bit channels. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== DM: support --config {f16,srgb}, create 16-bit PNGs. Will need to follow up with enabling f16 and srgb configs. They're still assert-y. When this lands, all new .pngs uploaded to Gold will have 16-bit channels. But the hashes for existing configs (8888, gpu, etc.) won't change, and the 16-bit images should compare identically to the old 8-bit images. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
mtklein@google.com changed reviewers: + mtklein@google.com
Please take another look. I've added --config srgb too. I think I'm doing the conversion from 8-bit PM sRGB to 16-bit UPM linear wrong. It should probably go via floats rather than via 8-bit linear. What's my best option to do sRGB 8888 -> linear ffff?
Oh wait, I think I'm double counting. We need to either mark the .png as sRGB, or convert it to linear 16, but not both. I think I'd prefer to convert to linear 16, for consistency with all the other configs.
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/240001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/240001
Okay, sorry for that false start. PTAL at PS 13, which now produces 16-bit linear PNGs for sRGB bitmaps. One thing to note is that when I was doing my conversion wrong, 8888 and f16 drew very similarly, where the sRGB images had major diffs (e.g. on the gradients GM). Now that I'm linearizing the sRGB correctly, they're all much more similar, mostly with different grey levels. This makes me wonder if those major differences you're seeing in SampleApp for sRGB are due to some part of that system misinterpreting the sRGB data as linear. I can certainly say I don't understand this all yet.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/260001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/260001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
The CQ bit was checked by mtklein@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/280001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/280001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from
==========
DM: support --config {f16,srgb}, create 16-bit PNGs.
Will need to follow up with enabling f16 and srgb configs. They're still
assert-y.
When this lands, all new .pngs uploaded to Gold will have 16-bit channels. But
the hashes for existing configs (8888, gpu, etc.) won't change, and the 16-bit
images should compare identically to the old 8-bit images.
BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========
to
==========
DM: support --config {f16,srgb}
Will need to follow up with enabling f16 and srgb configs.
They're still assert-y.
The GMs 'gamma' and 'gradients' look reassuringly good on f16 and srgb backends.
BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========
The CQ bit was checked by mtklein@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/360001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/360001
mtklein@chromium.org changed reviewers: + brianosman@google.com - mtklein@google.com, stephana@google.com
Please take (another) look. PS 19 now does what we settled on earlier this morning: - outputs PNGs stay 8-bit - for sRGB buffers, unpremultiply and shuffle to RGBA order - for f16 buffers, unpremultiply and pack to sRGB. - leave other buffers (linear 8888, 565) unchanged.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mtklein@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1711143002/360001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1711143002/360001
Message was sent while issue was closed.
Description was changed from
==========
DM: support --config {f16,srgb}
Will need to follow up with enabling f16 and srgb configs.
They're still assert-y.
The GMs 'gamma' and 'gradients' look reassuringly good on f16 and srgb backends.
BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========
to
==========
DM: support --config {f16,srgb}
Will need to follow up with enabling f16 and srgb configs.
They're still assert-y.
The GMs 'gamma' and 'gradients' look reassuringly good on f16 and srgb backends.
BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
Committed:
https://skia.googlesource.com/skia/+/27c3fddaac722086b80a7725f42c14fb694ec4a6
==========
Message was sent while issue was closed.
Committed patchset #19 (id:360001) as https://skia.googlesource.com/skia/+/27c3fddaac722086b80a7725f42c14fb694ec4a6 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
