Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2128893002:
Move sRGB <-> linear conversion components to their own files. (Closed)
DescriptionMove sRGB <-> linear conversion components to their own files.
This makes them a little easier to use outside SkColorXform code.
I've added some notes about how best to use them and their eccentricities, and added a test.
Ultimately any software sRGB <-> linear conversion should funnel somehow through here.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002
CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot
Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c
Committed: https://skia.googlesource.com/skia/+/ac41bac40f5a80d2bc5ccec584c23478a6900179
Patch Set 1 #Patch Set 2 : missing include #Patch Set 3 : move linear->srgb too #Patch Set 4 : tweak comments #Patch Set 5 : clarify? #
Total comments: 1
Patch Set 6 : relax test a little when NEON is present #Patch Set 7 : clearer #
Messages
Total messages: 55 (24 generated)
Description was changed from ========== Move sk_linear_from_srgb to its own .cpp/.h. This makes it a little easier to use outside SkColorXform code. BUG=skia: ========== to ========== Move sk_linear_from_srgb to its own .cpp/.h. This makes it a little easier to use outside SkColorXform code. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
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/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
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-SKNX_NO_SIMD-Trybot on client.skia (JOB_FAILED, http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)
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/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Move sk_linear_from_srgb to its own .cpp/.h. This makes it a little easier to use outside SkColorXform code. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
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/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
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/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ==========
mtklein@chromium.org changed reviewers: + msarett@google.com, reed@google.com
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
https://codereview.chromium.org/2128893002/diff/80001/src/opts/SkColorXform_o... File src/opts/SkColorXform_opts.h (left): https://codereview.chromium.org/2128893002/diff/80001/src/opts/SkColorXform_o... src/opts/SkColorXform_opts.h:29: static Sk4f linear_to_srgb(const Sk4f& x) { FWIW, I'm thinking about/working on refactoring this function signature to be more flexible... I like this being interchangeable with other "linear to curve" functions. It's a bad match when we want to use a table to go "linear to curve" because that must be written as Sk4f -> Sk4i (when planar) or Sk4f -> uint32_t (when interleaved).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
On 2016/07/07 17:05:35, msarett wrote: > https://codereview.chromium.org/2128893002/diff/80001/src/opts/SkColorXform_o... > File src/opts/SkColorXform_opts.h (left): > > https://codereview.chromium.org/2128893002/diff/80001/src/opts/SkColorXform_o... > src/opts/SkColorXform_opts.h:29: static Sk4f linear_to_srgb(const Sk4f& x) { > FWIW, I'm thinking about/working on refactoring this function signature to be > more flexible... > > I like this being interchangeable with other "linear to curve" functions. It's > a bad match when we want to use a table to go "linear to curve" because that > must be written as Sk4f -> Sk4i (when planar) or Sk4f -> uint32_t (when > interleaved). I, for one, like the flexibility we've got here. I'm planning on using it both for big 4 pixel slabs of R,G,B and also for RGBx one at a time when I've got <4 pixels.
Yeah I finally realized that my change didn't need to (or want to) change the signature. Thus, lgtm.
The CQ bit was checked by mtklein@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
Message was sent while issue was closed.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/2131793002/ by mtklein@google.com. The reason for reverting is: Monotonicity assert is failing on ARM. (Different rsqrt() and invert() precision?) Will investigate a bit tomorrow... might reland with the test TODO..
Message was sent while issue was closed.
On 2016/07/08 01:33:56, mtklein wrote: > A revert of this CL (patchset #5 id:80001) has been created in > https://codereview.chromium.org/2131793002/ by mailto:mtklein@google.com. > > The reason for reverting is: Monotonicity assert is failing on ARM. (Different > rsqrt() and invert() precision?) Will investigate a bit tomorrow... might > reland with the test TODO.. This is weird... there's only one step backwards causing the monotonicitiy assert to fail, and it's nowhere special. All the float values are approximately 0.166016, so I'll just list the hex: ... 3e29ffff -> 72 3e2a0000 -> 71 ... 3e2a0028 -> 71 3e2a0029 -> 72 ... This doesn't seem like a big deal, so I'm gonna reland allowing one regression in the test on NEON devices.
Message was sent while issue was closed.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot;client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ==========
Message was sent while issue was closed.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot;client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ==========
Message was sent while issue was closed.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ==========
The CQ bit was checked by mtklein@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from msarett@google.com Link to the patchset: https://codereview.chromium.org/2128893002/#ps120001 (title: "clearer")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
The CQ bit was unchecked by mtklein@google.com
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c COMMIT=false Can wait until next week. ==========
On 2016/07/08 12:13:54, mtklein wrote: > On 2016/07/08 01:33:56, mtklein wrote: > > A revert of this CL (patchset #5 id:80001) has been created in > > https://codereview.chromium.org/2131793002/ by mailto:mtklein@google.com. > > > > The reason for reverting is: Monotonicity assert is failing on ARM. > (Different > > rsqrt() and invert() precision?) Will investigate a bit tomorrow... might > > reland with the test TODO.. > > This is weird... there's only one step backwards causing the monotonicitiy > assert to fail, and it's nowhere special. All the float values are > approximately 0.166016, so I'll just list the hex: > > ... > 3e29ffff -> 72 > 3e2a0000 -> 71 > ... > 3e2a0028 -> 71 > 3e2a0029 -> 72 > ... > > This doesn't seem like a big deal, so I'm gonna reland allowing one regression > in the test on NEON devices. I'll fix this. I haven't really had the chance to look at the NEON at all, so I'm not completely surprised.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c COMMIT=false Can wait until next week. ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ==========
The CQ bit was checked by mtklein@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
There were warnings when CQ was processing your CL: * CQ_EXTRA_TRYBOTS flag is deprecated and will soon be removed. Use CQ_INCLUDE_TRYBOTS instead.
Message was sent while issue was closed.
Description was changed from ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c ========== to ========== Move sRGB <-> linear conversion components to their own files. This makes them a little easier to use outside SkColorXform code. I've added some notes about how best to use them and their eccentricities, and added a test. Ultimately any software sRGB <-> linear conversion should funnel somehow through here. BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2128893002 CQ_EXTRA_TRYBOTS=client.skia.android:Test-Android-GCC-Nexus5-CPU-NEON-Arm7-Release-Trybot;client.skia:Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-SKNX_NO_SIMD-Trybot Committed: https://skia.googlesource.com/skia/+/45e58c8807179638980aae8503573b950b844e4c Committed: https://skia.googlesource.com/skia/+/ac41bac40f5a80d2bc5ccec584c23478a6900179 ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://skia.googlesource.com/skia/+/ac41bac40f5a80d2bc5ccec584c23478a6900179 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
