976184 - canvasmark "click here" text rendered incorrectly

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: canvasmark-text.png

With the latest Skia rebase (bug 910754), the "click or press space to run canvasmark" message at the beginning is rendered incorrectly.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

We need a test case for whatever operation this is that keeps being problematic (fillText?).

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

:snorp, if you need this in 30, you're probably not going to get it out of George...

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

The following small test case reproduces the problem. It seems that drawing text with a shadow is completely busted. Surely we have a test for this?

var c = document.getElementById('c');
var ctx = c.getContext('2d');

ctx.fillStyle = 'red'
ctx.fillRect(0, 0, c.width, c.height);

ctx.fillStyle = 'black';
ctx.font = '12pt Arial';
ctx.shadowColor = 'white';
ctx.shadowOffsetX = 5;
ctx.shadowOffsetY = 5;
ctx.shadowBlur = 2;
ctx.fillText("Test", 50, 50);

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

It's busted with transparent images too.

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Looks like we still need the patch from bug 884888.

Comment 6

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Milan Sreckovic [:milan] from comment #2)
> :snorp, if you need this in 30, you're probably not going to get it out of
> George...

With comment 3 and 5 in mind, George will aim to fix it in 30.

Comment 7

[James Willcox (:snorp) ([e-mail]) (he/him)]

Looks like due to bug 907351, we never use SkiaGL during reftests at all, even for once that explicitly exercise the canvas. Ugh.

Comment 8

[George Wright (:gw280) (needinfo me!)]

Oh, I forgot that we weren't testing SkiaGL. Snorp and I had actually been discussing whether or not shadows were still broken. I will endeavour to get this fixed today as it shouldn't be too hard.

Comment 9

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-976184-Apply-a-drop-shadow-image-filter-to-our-S.patch

Comment 10

[George Wright (:gw280) (needinfo me!)]

https://tbpl.mozilla.org/?tree=Try&rev=18c1fdf16b88

Comment 11

[George Wright (:gw280) (needinfo me!)]

Comment on attachment 8387072 [details] [diff] [review]
0001-Bug-976184-Apply-a-drop-shadow-image-filter-to-our-S.patch

Matt's probably a better reviewer for this actually...

Comment 12

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8387072 [details] [diff] [review]
0001-Bug-976184-Apply-a-drop-shadow-image-filter-to-our-S.patch

Review of attachment 8387072 [details] [diff] [review]:
-----------------------------------------------------------------

Much happy.

Comment 13

[George Wright (:gw280) (needinfo me!)]

Attachment: 0001-Bug-976184-Apply-a-drop-shadow-image-filter-to-our-S.patch

Oops, didn't realise DrawSurfaceWithShadow operates in device space. Let's fix that. :)

Comment 14

[George Wright (:gw280) (needinfo me!)]

https://tbpl.mozilla.org/?tree=Try&rev=2eaf008d5bb8

Comment 16

[George Wright (:gw280) (needinfo me!)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/947d7ce90e26

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/947d7ce90e26