SVG Text decoration should respect non-scaling-stroke.

SVG Text decoration should respect non-scaling-stroke.

Stroke thickness of SVG text decoration should depend on non-scaling-stroke.

BUG=475203
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194749

[Shanmuga Pandi, 2015-04-27 14:23:05]

shanmuga.m@samsung.com changed reviewers:
+ ed@opera.com, fs@opera.com, pdr@chromium.org

[Shanmuga Pandi, 2015-04-27 14:23:06]

Please review it. Thanks

[fs, 2015-04-27 14:58:30]

This most likely doesn't work correctly for non-uniform scaling. But thinking
about it, that's probably true for the fix that was made to text, so I suppose
I'd be fine with calling this a progression. What do you think?

[pdr., 2015-04-27 20:22:08]

On 2015/04/27 at 14:58:30, fs wrote:
> This most likely doesn't work correctly for non-uniform scaling. But thinking
about it, that's probably true for the fix that was made to text, so I suppose
I'd be fine with calling this a progression. What do you think?

What's an example of non-uniform scaling? Non-affine transforms?

[pdr., 2015-04-27 20:22:32]

On 2015/04/27 at 20:22:08, pdr wrote:
> On 2015/04/27 at 14:58:30, fs wrote:
> > This most likely doesn't work correctly for non-uniform scaling. But
thinking about it, that's probably true for the fix that was made to text, so I
suppose I'd be fine with calling this a progression. What do you think?
> 
> What's an example of non-uniform scaling? Non-affine transforms?

Fwiw, I think this change looks good, just waiting for fs's response.

[fs, 2015-04-28 07:42:15]

On 2015/04/27 20:22:08, pdr wrote:
> On 2015/04/27 at 14:58:30, fs wrote:
> > This most likely doesn't work correctly for non-uniform scaling. But
thinking
> about it, that's probably true for the fix that was made to text, so I suppose
> I'd be fine with calling this a progression. What do you think?
> 
> What's an example of non-uniform scaling? Non-affine transforms?

scale(4 2)

[Shanmuga Pandi, 2015-04-28 10:01:15]

On 2015/04/28 07:42:15, fs wrote:
> On 2015/04/27 20:22:08, pdr wrote:
> > On 2015/04/27 at 14:58:30, fs wrote:
> > > This most likely doesn't work correctly for non-uniform scaling. But
> thinking
> > about it, that's probably true for the fix that was made to text, so I
suppose
> > I'd be fine with calling this a progression. What do you think?
> > 
> > What's an example of non-uniform scaling? Non-affine transforms?
> 
> scale(4 2)

I have tried to apply scale(4 2) and I am seeing same result as gecko.
In this case I am getting scalingFactor = 3.1622777 in
blink::SVGInlineTextBoxPainter::paintDecoration.

[fs, 2015-04-28 10:36:25]

On 2015/04/28 10:01:15, Shanmuga Pandi wrote:
> On 2015/04/28 07:42:15, fs wrote:
> > On 2015/04/27 20:22:08, pdr wrote:
> > > On 2015/04/27 at 14:58:30, fs wrote:
> > > > This most likely doesn't work correctly for non-uniform scaling. But
> > thinking
> > > about it, that's probably true for the fix that was made to text, so I
> suppose
> > > I'd be fine with calling this a progression. What do you think?
> > > 
> > > What's an example of non-uniform scaling? Non-affine transforms?
> > 
> > scale(4 2)
> 
> I have tried to apply scale(4 2) and I am seeing same result as gecko.
> In this case I am getting scalingFactor = 3.1622777 in
> blink::SVGInlineTextBoxPainter::paintDecoration.

If I were to test this out, I'd probably use scale factors that were
significantly (an order of magnitude) larger, or the non-uniformity will not be
so apparent.

[Shanmuga Pandi, 2015-04-28 11:29:01]

On 2015/04/28 10:36:25, fs wrote:
> On 2015/04/28 10:01:15, Shanmuga Pandi wrote:
> > On 2015/04/28 07:42:15, fs wrote:
> > > On 2015/04/27 20:22:08, pdr wrote:
> > > > On 2015/04/27 at 14:58:30, fs wrote:
> > > > > This most likely doesn't work correctly for non-uniform scaling. But
> > > thinking
> > > > about it, that's probably true for the fix that was made to text, so I
> > suppose
> > > > I'd be fine with calling this a progression. What do you think?
> > > > 
> > > > What's an example of non-uniform scaling? Non-affine transforms?
> > > 
> > > scale(4 2)
> > 
> > I have tried to apply scale(4 2) and I am seeing same result as gecko.
> > In this case I am getting scalingFactor = 3.1622777 in
> > blink::SVGInlineTextBoxPainter::paintDecoration.
> 
> If I were to test this out, I'd probably use scale factors that were
> significantly (an order of magnitude) larger, or the non-uniformity will not
be
> so apparent.


with stroke-width=2 and scale(100 2), I am getting below values
scalingFactor = 70.7248
strokethickness/scalingFactor = 0.0282786. 

This effect will be same for text and text-decoration.

[Shanmuga Pandi, 2015-04-30 12:15:27]

On 2015/04/28 11:29:01, Shanmuga Pandi wrote:
> On 2015/04/28 10:36:25, fs wrote:
> > On 2015/04/28 10:01:15, Shanmuga Pandi wrote:
> > > On 2015/04/28 07:42:15, fs wrote:
> > > > On 2015/04/27 20:22:08, pdr wrote:
> > > > > On 2015/04/27 at 14:58:30, fs wrote:
> > > > > > This most likely doesn't work correctly for non-uniform scaling. But
> > > > thinking
> > > > > about it, that's probably true for the fix that was made to text, so I
> > > suppose
> > > > > I'd be fine with calling this a progression. What do you think?
> > > > > 
> > > > > What's an example of non-uniform scaling? Non-affine transforms?
> > > > 
> > > > scale(4 2)
> > > 
> > > I have tried to apply scale(4 2) and I am seeing same result as gecko.
> > > In this case I am getting scalingFactor = 3.1622777 in
> > > blink::SVGInlineTextBoxPainter::paintDecoration.
> > 
> > If I were to test this out, I'd probably use scale factors that were
> > significantly (an order of magnitude) larger, or the non-uniformity will not
> be
> > so apparent.
> 
> 
> with stroke-width=2 and scale(100 2), I am getting below values
> scalingFactor = 70.7248
> strokethickness/scalingFactor = 0.0282786. 
> 
> This effect will be same for text and text-decoration.

Any comments on future of this patch is appreciated. Thank you.

[fs, 2015-04-30 12:21:22]

On 2015/04/30 12:15:27, Shanmuga Pandi wrote:
> On 2015/04/28 11:29:01, Shanmuga Pandi wrote:
> > On 2015/04/28 10:36:25, fs wrote:
> > > On 2015/04/28 10:01:15, Shanmuga Pandi wrote:
> > > > On 2015/04/28 07:42:15, fs wrote:
> > > > > On 2015/04/27 20:22:08, pdr wrote:
> > > > > > On 2015/04/27 at 14:58:30, fs wrote:
> > > > > > > This most likely doesn't work correctly for non-uniform scaling.
But
> > > > > thinking
> > > > > > about it, that's probably true for the fix that was made to text, so
I
> > > > suppose
> > > > > > I'd be fine with calling this a progression. What do you think?
> > > > > > 
> > > > > > What's an example of non-uniform scaling? Non-affine transforms?
> > > > > 
> > > > > scale(4 2)
> > > > 
> > > > I have tried to apply scale(4 2) and I am seeing same result as gecko.
> > > > In this case I am getting scalingFactor = 3.1622777 in
> > > > blink::SVGInlineTextBoxPainter::paintDecoration.
> > > 
> > > If I were to test this out, I'd probably use scale factors that were
> > > significantly (an order of magnitude) larger, or the non-uniformity will
not
> > be
> > > so apparent.
> > 
> > 
> > with stroke-width=2 and scale(100 2), I am getting below values
> > scalingFactor = 70.7248
> > strokethickness/scalingFactor = 0.0282786. 
> > 
> > This effect will be same for text and text-decoration.
> 
> Any comments on future of this patch is appreciated. Thank you.

As I said earlier, I'm fine with landing this since it certainly is a
progression. It should be know that there's still is bugs here though.

lgtm as such

[Shanmuga Pandi, 2015-04-30 12:49:04]

On 2015/04/30 12:21:22, fs wrote:
> On 2015/04/30 12:15:27, Shanmuga Pandi wrote:
> > On 2015/04/28 11:29:01, Shanmuga Pandi wrote:
> > > On 2015/04/28 10:36:25, fs wrote:
> > > > On 2015/04/28 10:01:15, Shanmuga Pandi wrote:
> > > > > On 2015/04/28 07:42:15, fs wrote:
> > > > > > On 2015/04/27 20:22:08, pdr wrote:
> > > > > > > On 2015/04/27 at 14:58:30, fs wrote:
> > > > > > > > This most likely doesn't work correctly for non-uniform scaling.
> But
> > > > > > thinking
> > > > > > > about it, that's probably true for the fix that was made to text,
so
> I
> > > > > suppose
> > > > > > > I'd be fine with calling this a progression. What do you think?
> > > > > > > 
> > > > > > > What's an example of non-uniform scaling? Non-affine transforms?
> > > > > > 
> > > > > > scale(4 2)
> > > > > 
> > > > > I have tried to apply scale(4 2) and I am seeing same result as gecko.
> > > > > In this case I am getting scalingFactor = 3.1622777 in
> > > > > blink::SVGInlineTextBoxPainter::paintDecoration.
> > > > 
> > > > If I were to test this out, I'd probably use scale factors that were
> > > > significantly (an order of magnitude) larger, or the non-uniformity will
> not
> > > be
> > > > so apparent.
> > > 
> > > 
> > > with stroke-width=2 and scale(100 2), I am getting below values
> > > scalingFactor = 70.7248
> > > strokethickness/scalingFactor = 0.0282786. 
> > > 
> > > This effect will be same for text and text-decoration.
> > 
> > Any comments on future of this patch is appreciated. Thank you.
> 
> As I said earlier, I'm fine with landing this since it certainly is a
> progression. It should be know that there's still is bugs here though.
> 
> lgtm as such

Thank you very much!!

[Shanmuga Pandi, 2015-04-30 12:49:12]

The CQ bit was checked by shanmuga.m@samsung.com

[commit-bot: I haz the power, 2015-04-30 12:49:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1107983003/1

[commit-bot: I haz the power, 2015-04-30 14:16:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-30 14:16:03]

Failed to apply patch for LayoutTests/TestExpectations:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file LayoutTests/TestExpectations
  Hunk #1 FAILED at 2121.
  1 out of 1 hunk FAILED -- saving rejects to file
LayoutTests/TestExpectations.rej

Patch:       LayoutTests/TestExpectations
Index: LayoutTests/TestExpectations
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
index
8ce53a9bf7bb8c7c93efed7b3f9f96789f968991..e426a536a425c2d7d03856da8b8b4956c5220076
100644
--- a/LayoutTests/TestExpectations
+++ b/LayoutTests/TestExpectations
@@ -2121,3 +2121,5 @@ crbug.com/479181
inspector-protocol/animation/animation-multiple-frames.html [ P
 
 crbug.com/479202 fast/forms/select/select-typeahead-with-spacekey.html [ Pass
Failure ]
 crbug.com/479202
virtual/slimmingpaint/fast/forms/select/select-typeahead-with-spacekey.html [
Pass Failure ]
+
+crbug.com/475203 svg/repaint/repaint-non-scaling-stroke-text-decoration.html [
NeedsRebaseline ]

[commit-bot: I haz the power, 2015-04-30 14:16:11]

The CQ bit was unchecked by commit-bot@chromium.org

[Shanmuga Pandi, 2015-04-30 14:39:40]

The CQ bit was checked by shanmuga.m@samsung.com

[Shanmuga Pandi, 2015-04-30 14:39:40]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com
Link to the patchset: https://codereview.chromium.org/1107983003/#ps20001
(title: "rebased")

[commit-bot: I haz the power, 2015-04-30 14:40:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1107983003/20001

[commit-bot: I haz the power, 2015-04-30 16:15:37]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194749