createjs.BlurFilter defect when AzureCanvasBackend are direct2d

RESOLVED FIXED in Firefox 54

Status

()

Core
Canvas: 2D
P1
normal
RESOLVED FIXED
8 months ago
8 months ago

People

(Reporter: devguider, Assigned: lsalzman)

Tracking

({regression, testcase})

54 Branch
mozilla55
All
Windows
regression, testcase
Points:
---

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox52 unaffected, firefox-esr52 unaffected, firefox53 unaffected, firefox54+ fixed, firefox55+ fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 months ago
Created attachment 8847099 [details]
ff54bug.rar

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170314004020

Steps to reproduce:

Test the submitted demo.


Actual results:

The blurred element is being stretched and position out of place on blur effect.


Expected results:

The blurred element should preserve it's properties, only the blurred effect can be applied.

Comment 1

8 months ago
Why is it not the expected effect? How to reproduce the expected effect?
Component: Untriaged → Untriaged
Flags: needinfo?(devguider)
Keywords: testcase
Product: Firefox → Core
Summary: html 5 createjs blur defect → createjs.BlurFilter defect
(Reporter)

Comment 2

8 months ago
The problem is valid only for ff54 in all other browsers work fine without change the x position.

The expected effect image can't be change positions only blur effect can be applied.

Comment 3

8 months ago
I see it is different cases in same version with different profile.

Normal if:
AzureCanvasBackend	skia
AzureContentBackend	skia
AzureFallbackCanvasBackend	cairo

Offset if:
AzureCanvasBackend	direct2d 1.1
AzureContentBackend	direct2d 1.1
AzureFallbackCanvasBackend	skia


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=3b7d96464366abd6b5a8c45cfec0444510d2f602&tochange=0c3de6069e8efdce7edf84231fcc5a4b09368885
Blocks: 1334749
Status: UNCONFIRMED → NEW
Has Regression Range: --- → yes
Has STR: --- → yes
status-firefox52: --- → unaffected
status-firefox53: --- → unaffected
status-firefox54: --- → affected
status-firefox55: --- → affected
Component: Untriaged → Canvas: 2D
Ever confirmed: true
Flags: needinfo?(devguider)
Keywords: regression
OS: Unspecified → Windows 10
Hardware: Unspecified → x86
Summary: createjs.BlurFilter defect → createjs.BlurFilter defect while gfx.canvas.azure.backends are direct2d
Hmm.  I have no idea why this would even be backend-dependent.  Lee, do you have any idea?
Flags: needinfo?(lsalzman)
tracking-firefox54: --- → ?
tracking-firefox55: --- → ?
Tracking 54/55+ to investigate this regression.
tracking-firefox54: ? → +
tracking-firefox55: ? → +
YF, do you see this on 55 too, or just on 54?
Flags: needinfo?(yfdyh000)

Comment 7

8 months ago
(In reply to Boris Zbarsky [:bz] (still a bit busy) (if a patch has no decent message, automatic r-) from comment #6)
> YF, do you see this on 55 too, or just on 54?

55 too. Version 55.0a1, Build ID 20170314030215.
Flags: needinfo?(yfdyh000)
(Assignee)

Comment 8

8 months ago
Created attachment 8847368 [details] [diff] [review]
fix DrawTargetD2D1::GetImageForSurface to support uploads in device-space

CopySurface is supposed to operate in device-space. Unfortunately, DrawTargetD2D1::GetImageForSurface always reads from the user-space transform (as per https://dxr.mozilla.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp?q=DrawTargetD2D1%3A%3AGetImageForSurface&redirect_type=single#1879).

This causes problems when DrawTargetD2D1::CopySurface attempts to clear the transform on the device context (https://dxr.mozilla.org/mozilla-central/source/gfx/2d/DrawTargetD2D1.cpp?q=DrawTargetD2D1%3A%3ACopySurface&redirect_type=single#381). The partial upload inside GetImageForSurface is basically trying to clip the partial upload in user-space, which is wrong.

We need to ignore this transform for this to properly clip the uploaded surface. This patch does that, which was simpler that the alternatives.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Flags: needinfo?(lsalzman)
Attachment #8847368 - Flags: review?(mchang)
Attachment #8847368 - Flags: review?(mchang) → review+

Updated

8 months ago
Summary: createjs.BlurFilter defect while gfx.canvas.azure.backends are direct2d → createjs.BlurFilter defect when AzureCanvasBackend are direct2d
(Assignee)

Comment 9

8 months ago
Created attachment 8847389 [details] [diff] [review]
fix DrawTargetD2D1::GetImageForSurface to support uploads in device-space. r=mchang

Added a simplified reftest to make sure this doesn't happen in the future.
Attachment #8847368 - Attachment is obsolete: true
Attachment #8847389 - Flags: review+

Comment 10

8 months ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fa6e1037d65d
fix DrawTargetD2D1::GetImageForSurface to support uploads in device-space. r=mchang
(Assignee)

Updated

8 months ago
status-firefox-esr45: --- → unaffected
status-firefox-esr52: --- → unaffected
OS: Windows 10 → Windows
Priority: -- → P1
Hardware: x86 → All
Whiteboard: [gfx-noted]

Comment 11

8 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fa6e1037d65d
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Comment 12

8 months ago
Comment on attachment 8847389 [details] [diff] [review]
fix DrawTargetD2D1::GetImageForSurface to support uploads in device-space. r=mchang

Approval Request Comment
[Feature/Bug causing the regression]: bug 1334749
[User impact if declined]: Canvas2D putImageData will not be positioned correctly on Windows. 
[Is this code covered by automated tests?]: yes, added a new test for this
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: just this
[Is the change risky?]:  no
[Why is the change risky/not risky?]: Just a small change to ensure that the draw target's transformation matrix should not be applied in places it shouldn't have been. Fixes user's testcase and has added reftest to verify nothing broken and everything works.
[String changes made/needed]: none
Attachment #8847389 - Flags: approval-mozilla-aurora?
Comment on attachment 8847389 [details] [diff] [review]
fix DrawTargetD2D1::GetImageForSurface to support uploads in device-space. r=mchang

Fix a regression and include a test. Aurora54+.
Attachment #8847389 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 14

8 months ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-aurora/rev/b6b6a603722a
status-firefox54: affected → fixed
You need to log in before you can comment on or make changes to this bug.