Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2187083002:
Add initial CurveMeasure code (Closed)
|
Created:
4 years, 4 months ago by Harry Stern Modified:
4 years, 4 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionAdd initial CurveMeasure code
- This code is entirely private and is not being used by anything.
- In a future CL we will write a class that uses CurveMeasure to compute dash points. In order to determine whether CurveMeasure or PathMeasure should be faster, we need the dash info (the sum of the on/off intervals and how many there are)
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
Committed: https://skia.googlesource.com/skia/+/0446a3c8e2a5f1d525b3e0595252e64c6cd3a753
Patch Set 1 #Patch Set 2 : Implement evaluateQuad and evaluateQuadDerivative #
Total comments: 4
Patch Set 3 : Make various array parameters const #Patch Set 4 : Add comment explaining weights and abscissa arrays #Patch Set 5 : Fix build errors on windows #Patch Set 6 : Fix some destructor weirdness because of the virtuals #Patch Set 7 : Add initial measure benches #Patch Set 8 : Refactor to get rid of the virtuals, use a switch on an enum for the curve type #Patch Set 9 : Remove unused variable #
Total comments: 6
Patch Set 10 : Use SK_SIZE_T_SPECIFIER for printing size_t in bench #Patch Set 11 : Use SK_VECTORCALL when passing Sk8f arguments #Patch Set 12 : Various formatting changes #Patch Set 13 : Add heuristic to normalize the number of inner loops in MeasureBench #Patch Set 14 : Minor formatting changes #Patch Set 15 : Change SkDebugf to SkDEBUGF #Patch Set 16 : Make NORMALIZE_LOOPS a define since it's only for developer testing #Patch Set 17 : Move evaluateDerivativeLength to a static function and put coefficients in array #Patch Set 18 : Revert use of cubic in MeasureBench #
Total comments: 2
Patch Set 19 : Unindent public:/private: #Patch Set 20 : Rebase off master #
Total comments: 1
Patch Set 21 : Include <algorithm> for overloaded min/max #
Messages
Total messages: 53 (27 generated)
Description was changed from
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally is SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
==========
to
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally is SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
Description was changed from
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally is SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
to
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally is SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
hstern@google.com changed reviewers: + caryclark@google.com, mtklein@google.com, reed@google.com
So right now this code just does quads, but with only a tiny bit of work it should be usable for cubics and conics (and special casing for lines) as well. I'm not sure if I should leave the quadrature code in the header vs putting it in the .cpp file. In c++ is the only difference that the headers are compiled every time, so it increases compile time slightly? Usually I just decide based on the actual length of the code. I will need some help figuring out how to do the templating correctly so that we don't have to have the virtuals for the integrator/evaluator subclasses. Also, as in the description, I'm not sure how to write a bench for code that doesn't have a public api.
My suggestion is remove cubic scaffolding for now. Will be interesting when we also see benches to see if making methods virtual hurts much https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasure.h File src/utils/SkCurveMeasure.h (right): https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasur... src/utils/SkCurveMeasure.h:14: static SkScalar weights8[8] = {0.3626837833783620, 0.3626837833783620, 0.3137066458778873, 0.3137066458778873, 0.2223810344533745, 0.2223810344533745, 0.1012285362903763, 0.1012285362903763}; can you move these into private section of ArcLengthIntegrator? https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasur... src/utils/SkCurveMeasure.h:75: Sk8f evaluate_derivative_x(Sk8f &ts) override { const Sk8f& ts
Description was changed from
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally is SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
to
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally in SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
On 2016/07/27 19:59:43, reed1 wrote: > My suggestion is remove cubic scaffolding for now. What do you mean? just the commented lines for evaluate_cubic_derivative and stuff?
https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasure.h File src/utils/SkCurveMeasure.h (right): https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasur... src/utils/SkCurveMeasure.h:14: static SkScalar weights8[8] = {0.3626837833783620, 0.3626837833783620, 0.3137066458778873, 0.3137066458778873, 0.2223810344533745, 0.2223810344533745, 0.1012285362903763, 0.1012285362903763}; On 2016/07/27 19:59:43, reed1 wrote: > can you move these into private section of ArcLengthIntegrator? I can, but the Sk8f weights and absc would have to be initialized in the constructor for each Integrator because SkNx has non-constexpr constructors, from what the compiler seems to be telling me. It's not a big deal since it's only 8 floats, so I can do it if you want. I don't really understand why they should be, though. I should probably rename them to kWeights and kAbsc. https://codereview.chromium.org/2187083002/diff/20001/src/utils/SkCurveMeasur... src/utils/SkCurveMeasure.h:75: Sk8f evaluate_derivative_x(Sk8f &ts) override { On 2016/07/27 19:59:43, reed1 wrote: > const Sk8f& ts Done.
I have initial benches. See https://paste.googleplex.com/4613472423772160?raw and https://paste.googleplex.com/5054831047737344?raw The CurveMeasure code is significantly slower on long paths with lots of pieces, but it is also significantly faster on long paths with very few pieces. This makes sense - once you do the linearization of the curve (in PathMeasure), finding the ts are just binary search and a few lerps. So we should only be faster when there are few enough pieces to compute that it takes less time than to linearize.
I have initial benches. See https://paste.googleplex.com/4613472423772160?raw and https://paste.googleplex.com/5054831047737344?raw The CurveMeasure code is significantly slower on long paths with lots of pieces, but it is also significantly faster on long paths with very few pieces. This makes sense - once you do the linearization of the curve (in PathMeasure), finding the ts are just binary search and a few lerps. So we should only be faster when there are few enough pieces to compute that it takes less time than to linearize.
On 2016/07/28 17:04:28, Harry Stern wrote: > I have initial benches. > > See https://paste.googleplex.com/4613472423772160?raw and > https://paste.googleplex.com/5054831047737344?raw > > The CurveMeasure code is significantly slower on long paths with lots of > pieces, but it is also significantly faster on long paths with very few > pieces. This makes sense - once you do the linearization of the curve > (in PathMeasure), finding the ts are just binary search and a few lerps. > > So we should only be faster when there are few enough pieces to compute > that it takes less time than to linearize. Can you create a function that chooses your code only when it is advantageous to do so?
On 2016/07/28 17:15:07, caryclark wrote: > On 2016/07/28 17:04:28, Harry Stern wrote: > > I have initial benches. > > > > See https://paste.googleplex.com/4613472423772160?raw and > > https://paste.googleplex.com/5054831047737344?raw > > > > The CurveMeasure code is significantly slower on long paths with lots of > > pieces, but it is also significantly faster on long paths with very few > > pieces. This makes sense - once you do the linearization of the curve > > (in PathMeasure), finding the ts are just binary search and a few lerps. > > > > So we should only be faster when there are few enough pieces to compute > > that it takes less time than to linearize. > > Can you create a function that chooses your code only when it is advantageous to > do so? Apparently I can't just reply-all on the email to get the reply to show up here. So I thought about it, and I think the answer is basically "no", because the API for dashing is not "draw me this many pieces", it's "draw pieces of this length until the whole thing is dashed", so there's not really any good way to know ahead of time how many pieces we will draw. So I really do need, like you said, a better algorithm rather than some optimization.
On 2016/07/28 17:51:47, Harry Stern wrote: > On 2016/07/28 17:15:07, caryclark wrote: > > On 2016/07/28 17:04:28, Harry Stern wrote: > > > I have initial benches. > > > > > > See https://paste.googleplex.com/4613472423772160?raw and > > > https://paste.googleplex.com/5054831047737344?raw > > > > > > The CurveMeasure code is significantly slower on long paths with lots of > > > pieces, but it is also significantly faster on long paths with very few > > > pieces. This makes sense - once you do the linearization of the curve > > > (in PathMeasure), finding the ts are just binary search and a few lerps. > > > > > > So we should only be faster when there are few enough pieces to compute > > > that it takes less time than to linearize. > > > > Can you create a function that chooses your code only when it is advantageous > to > > do so? > > Apparently I can't just reply-all on the email to get the reply to show up here. > > So I thought about it, and I think the answer is basically "no", because the API > for dashing is not "draw me this many pieces", it's "draw pieces of this length > until the whole thing is dashed", so there's not really any good way to know > ahead of time how many pieces we will draw. > > So I really do need, like you said, a better algorithm rather than some > optimization. I was thinking of a bounded approximation. - A curve is at least as long as the linear distance between endpoints. - A curve is never longer than the distance from the first endpoint to each control point to the last endpoint. - The length of the dash and gap determine the fewest and most number of pieces for a given curve, by dividing into the min and max described above. Does this give you the parameters you need to create a rough guess on when your function is faster?
Can we defer the work to decide which/when to use the code after have an initial land of the new code? I'd feel better having the new class be landed so Cary and I can play with it (and profile it), even if it is not hooked into the mainline yet.
The CQ bit was checked by hstern@google.com 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...
On 2016/08/02 20:59:54, reed1 wrote: > Can we defer the work to decide which/when to use the code after have an initial > land of the new code? I'd feel better having the new class be landed so Cary and > I can play with it (and profile it), even if it is not hooked into the mainline > yet. The bench does work currently (only for quadratics) and can be profiled, with stubs for line segments, cubics, and conics. This code is landable as is (modulo code review).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-Clang-x86_64-Debug-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-Clang-x...)
Description was changed from
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything, but
eventually it may be used internally in SkPathMeasure.
- I am unsure how to write a bench because it is private.
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
to
==========
Add initial CurveMeasure code
- This code is entirely private and is not being used by anything.
- In a future CL we will write a class that uses CurveMeasure to compute dash
points. In order to determine whether CurveMeasure or PathMeasure should be
faster, we need the dash info (the sum of the on/off intervals and how many
there are)
BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002
==========
The CQ bit was checked by hstern@google.com 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...
https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.cpp (right): https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:62: Sk8f ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { Windows buildbots are giving me "error C2719: 'ts': formal parameter with requested alignment of 16 won't be aligned" on this line and in the header. What does this mean?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Ubuntu-GCC-Arm7-Debug-Android-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Arm...) Build-Ubuntu-GCC-Mips-Debug-Android-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Ubuntu-GCC-Mip...)
https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.cpp (right): https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:13: ArcLengthIntegrator::ArcLengthIntegrator(const SkPoint* pts, SkSegType segType) : fSegType(segType) { exceed 100cols https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:62: Sk8f ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { On 2016/08/02 21:33:21, Harry Stern wrote: > Windows buildbots are giving me > "error C2719: 'ts': formal parameter with requested alignment of 16 won't be > aligned" on this line and in the header. What does this mean? Can you pass ts by const& ?
https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.cpp (right): https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:13: ArcLengthIntegrator::ArcLengthIntegrator(const SkPoint* pts, SkSegType segType) : fSegType(segType) { On 2016/08/02 21:46:23, reed1 wrote: > exceed 100cols Done, and various other formatting stuff. https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:62: Sk8f ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { On 2016/08/02 21:46:23, reed1 wrote: > On 2016/08/02 21:33:21, Harry Stern wrote: > > Windows buildbots are giving me > > "error C2719: 'ts': formal parameter with requested alignment of 16 won't be > > aligned" on this line and in the header. What does this mean? > > Can you pass ts by const& ? Mike Klein told me I might be able to use SK_VECTORCALL, and according to https://msdn.microsoft.com/en-us/library/dn375768.aspx that might additionally make it faster. Apparently __vectorcall makes it pass via registers. We'll see if that makes it stop complaining.
The CQ bit was checked by hstern@google.com 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.cpp (right): https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.cpp:62: Sk8f ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { On 2016/08/02 22:05:08, Harry Stern wrote: > On 2016/08/02 21:46:23, reed1 wrote: > > On 2016/08/02 21:33:21, Harry Stern wrote: > > > Windows buildbots are giving me > > > "error C2719: 'ts': formal parameter with requested alignment of 16 won't be > > > aligned" on this line and in the header. What does this mean? > > > > Can you pass ts by const& ? > > Mike Klein told me I might be able to use SK_VECTORCALL, and according to > https://msdn.microsoft.com/en-us/library/dn375768.aspx that might additionally > make it faster. Apparently __vectorcall makes it pass via registers. We'll see > if that makes it stop complaining. Actually I think I had a rather more nuanced response and I would like to reiterate that making functions that work with SkNx types static and inline is almost always the best option. If we _can_ make this a static function, it'd just take your argument by const&. Is this virtual or anything? Does it use ArcLengthIntegrator's state? Can you just pass a pointer to 8 floats instead of this?
On 2016/08/02 22:32:35, mtklein wrote: > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > File src/utils/SkCurveMeasure.cpp (right): > > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > src/utils/SkCurveMeasure.cpp:62: Sk8f > ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { > On 2016/08/02 22:05:08, Harry Stern wrote: > > On 2016/08/02 21:46:23, reed1 wrote: > > > On 2016/08/02 21:33:21, Harry Stern wrote: > > > > Windows buildbots are giving me > > > > "error C2719: 'ts': formal parameter with requested alignment of 16 won't > be > > > > aligned" on this line and in the header. What does this mean? > > > > > > Can you pass ts by const& ? > > > > Mike Klein told me I might be able to use SK_VECTORCALL, and according to > > https://msdn.microsoft.com/en-us/library/dn375768.aspx that might additionally > > make it faster. Apparently __vectorcall makes it pass via registers. We'll see > > if that makes it stop complaining. > > Actually I think I had a rather more nuanced response and I would like to > reiterate that making functions that work with SkNx types static and inline is > almost always the best option. If we _can_ make this a static function, it'd > just take your argument by const&. Is this virtual or anything? Does it use > ArcLengthIntegrator's state? Can you just pass a pointer to 8 floats instead of > this? I apologize for misrepresenting what you told me in chat. I don't think we can make this function static because it depends on the cached coefficients that are created in the constructor. In order to pass a pointer to 8 floats I'd have to un-SkNx the ts variable into an array and then put it back, because I want to use the SIMD operations inside this function.
On 2016/08/03 15:18:25, Harry Stern wrote: > On 2016/08/02 22:32:35, mtklein wrote: > > > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > > File src/utils/SkCurveMeasure.cpp (right): > > > > > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > > src/utils/SkCurveMeasure.cpp:62: Sk8f > > ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { > > On 2016/08/02 22:05:08, Harry Stern wrote: > > > On 2016/08/02 21:46:23, reed1 wrote: > > > > On 2016/08/02 21:33:21, Harry Stern wrote: > > > > > Windows buildbots are giving me > > > > > "error C2719: 'ts': formal parameter with requested alignment of 16 > won't > > be > > > > > aligned" on this line and in the header. What does this mean? > > > > > > > > Can you pass ts by const& ? > > > > > > Mike Klein told me I might be able to use SK_VECTORCALL, and according to > > > https://msdn.microsoft.com/en-us/library/dn375768.aspx that might > additionally > > > make it faster. Apparently __vectorcall makes it pass via registers. We'll > see > > > if that makes it stop complaining. > > > > Actually I think I had a rather more nuanced response and I would like to > > reiterate that making functions that work with SkNx types static and inline is > > almost always the best option. If we _can_ make this a static function, it'd > > just take your argument by const&. Is this virtual or anything? Does it use > > ArcLengthIntegrator's state? Can you just pass a pointer to 8 floats instead > of > > this? > > I apologize for misrepresenting what you told me in chat. I don't think we can > make this function static because it depends on the cached coefficients that are > created in the constructor. In order to pass a pointer to 8 floats I'd have to > un-SkNx the ts variable into an array and then put it back, because I want to > use the SIMD operations inside this function. I could make a static function outside the class with a type signature like `static inline Sk8f evaluateDerivativeLength(const Sk8f& coeff[4], const Sk8f& ts, const SkSegType segType);` Would that be good?
On 2016/08/03 16:04:51, Harry Stern wrote: > On 2016/08/03 15:18:25, Harry Stern wrote: > > On 2016/08/02 22:32:35, mtklein wrote: > > > > > > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > > > File src/utils/SkCurveMeasure.cpp (right): > > > > > > > > > https://codereview.chromium.org/2187083002/diff/160001/src/utils/SkCurveMeasu... > > > src/utils/SkCurveMeasure.cpp:62: Sk8f > > > ArcLengthIntegrator::evaluateDerivativeLength(Sk8f ts) { > > > On 2016/08/02 22:05:08, Harry Stern wrote: > > > > On 2016/08/02 21:46:23, reed1 wrote: > > > > > On 2016/08/02 21:33:21, Harry Stern wrote: > > > > > > Windows buildbots are giving me > > > > > > "error C2719: 'ts': formal parameter with requested alignment of 16 > > won't > > > be > > > > > > aligned" on this line and in the header. What does this mean? > > > > > > > > > > Can you pass ts by const& ? > > > > > > > > Mike Klein told me I might be able to use SK_VECTORCALL, and according to > > > > https://msdn.microsoft.com/en-us/library/dn375768.aspx that might > > additionally > > > > make it faster. Apparently __vectorcall makes it pass via registers. We'll > > see > > > > if that makes it stop complaining. > > > > > > Actually I think I had a rather more nuanced response and I would like to > > > reiterate that making functions that work with SkNx types static and inline > is > > > almost always the best option. If we _can_ make this a static function, > it'd > > > just take your argument by const&. Is this virtual or anything? Does it > use > > > ArcLengthIntegrator's state? Can you just pass a pointer to 8 floats > instead > > of > > > this? > > > > I apologize for misrepresenting what you told me in chat. I don't think we can > > make this function static because it depends on the cached coefficients that > are > > created in the constructor. In order to pass a pointer to 8 floats I'd have to > > un-SkNx the ts variable into an array and then put it back, because I want to > > use the SIMD operations inside this function. > > I could make a static function outside the class with a type signature like > `static inline Sk8f evaluateDerivativeLength(const Sk8f& coeff[4], const Sk8f& > ts, const SkSegType segType);` > > Would that be good? yes
I changed the benchmark to have a parameter which normalizes the number of inner loops (we do the inner loops because otherwise the benches run too fast to measure accurately). Here is the output from running locally: https://paste.googleplex.com/5085720184094720?raw https://paste.googleplex.com/4539251580469248?raw Normalizing has the disadvantage that we can't compare between benches which have one parameter constant and another increasing, but it runs much faster so we don't slow down perf bots.
lgtm w/ nits https://codereview.chromium.org/2187083002/diff/330001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.h (right): https://codereview.chromium.org/2187083002/diff/330001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.h:38: public: nit: skia does not indent public: or private:
https://codereview.chromium.org/2187083002/diff/330001/src/utils/SkCurveMeasu... File src/utils/SkCurveMeasure.h (right): https://codereview.chromium.org/2187083002/diff/330001/src/utils/SkCurveMeasu... src/utils/SkCurveMeasure.h:38: public: On 2016/08/05 21:49:08, reed1 wrote: > nit: skia does not indent public: or private: Done.
https://codereview.chromium.org/2187083002/diff/370001/gyp/utils.gypi File gyp/utils.gypi (right): https://codereview.chromium.org/2187083002/diff/370001/gyp/utils.gypi#newcode72 gyp/utils.gypi:72: '<(skia_src_path)/utils/SkShadowPaintFilterCanvas.cpp', These lines are here in both master and in my branch. Why are they showing up? I even did a rebase-update and a cl upload to see if that would fix it.
The CQ bit was checked by hstern@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com Link to the patchset: https://codereview.chromium.org/2187083002/#ps370001 (title: "Rebase off master")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: Build-Mac-Clang-x86_64-Release-GN-Trybot on master.client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Mac-Clang-x86_...)
The CQ bit was checked by hstern@google.com 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...
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 hstern@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from reed@google.com Link to the patchset: https://codereview.chromium.org/2187083002/#ps390001 (title: "Include <algorithm> for overloaded min/max")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add initial CurveMeasure code - This code is entirely private and is not being used by anything. - In a future CL we will write a class that uses CurveMeasure to compute dash points. In order to determine whether CurveMeasure or PathMeasure should be faster, we need the dash info (the sum of the on/off intervals and how many there are) BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002 ========== to ========== Add initial CurveMeasure code - This code is entirely private and is not being used by anything. - In a future CL we will write a class that uses CurveMeasure to compute dash points. In order to determine whether CurveMeasure or PathMeasure should be faster, we need the dash info (the sum of the on/off intervals and how many there are) BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2187083002 Committed: https://skia.googlesource.com/skia/+/0446a3c8e2a5f1d525b3e0595252e64c6cd3a753 ==========
Message was sent while issue was closed.
Committed patchset #21 (id:390001) as https://skia.googlesource.com/skia/+/0446a3c8e2a5f1d525b3e0595252e64c6cd3a753 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
