SVG content has wrong resolution in layers-free mode

RESOLVED FIXED in Firefox 58

Status

()

enhancement
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ethlin, Assigned: ethlin)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox56 unaffected, firefox57 unaffected, firefox58 fixed)

Details

(Whiteboard: [wr-mvp])

Attachments

(4 attachments)

Currently we use item's bounds as the draw target's size. But for svg, we should refer the svg viewport size. A related reftest is "layout/reftests/svg/as-image/canvas-drawImage-scale-2a.html".
Looks like we should do preScale for the draw target. Currently we store the scale value in the stacking context helper. I'll try to have a patch to address this problem for fallback. For "CreateWebrenderCommands", I'm not sure how to do this and also not sure if we need to do this.
The patch works but I need to fix the offset problem after scaling..
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [wr-mvp]
Target Milestone: --- → mozilla57
I'm not entirely convinced this is the right solution. How does this compare with what we're doing with layers today? How are we choosing the resolution there?
Flags: needinfo?(ethlin)
(In reply to Jeff Muizelaar [:jrmuizel] from comment #9)
> I'm not entirely convinced this is the right solution. How does this compare
> with what we're doing with layers today? How are we choosing the resolution
> there?

The FrameLayerBuilder uses similar way at [1]. The userData->mXScale should correspond to stacking context's mXScale.  

[1] https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/layout/painting/FrameLayerBuilder.cpp#6220
Flags: needinfo?(ethlin)
Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

https://reviewboard.mozilla.org/r/176764/#review186864
Attachment #8904954 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

https://reviewboard.mozilla.org/r/177560/#review186866
Attachment #8905759 - Flags: review?(jmuizelaar) → review+
Comment on attachment 8905754 [details]
Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

https://reviewboard.mozilla.org/r/177558/#review186868
Attachment #8905754 - Flags: review?(jmuizelaar) → review+
According to the try result[1], I should remove some fails-if. There are three new failures, and I should check them first.
 
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=203c7d3ee8c0acd7e631c1d329888418e8f7cf70&selectedJob=133508006
You shouldn't need part 3 anymore either since layers-free is on by default now.
Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

https://reviewboard.mozilla.org/r/176764/#review189738
Attachment #8904954 - Flags: review?(bugmail) → review+
Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

https://reviewboard.mozilla.org/r/177560/#review189740
Attachment #8905759 - Flags: review?(bugmail) → review+
Comment on attachment 8913143 [details]
Bug 1395501 - Part4. Update reftest annotations for new failures.

https://reviewboard.mozilla.org/r/184568/#review189742
Attachment #8913143 - Flags: review?(bugmail) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a505b6b021dd
Part1. Store inherited scale in stacking context. r=jrmuizel,kats
https://hg.mozilla.org/integration/autoland/rev/3b3e9f5671b2
Part2. Use stacking context's scale value to compute the correct paint rect. r=jrmuizel
https://hg.mozilla.org/integration/autoland/rev/75f20de3a189
Part3. Update reftest annotations for unexpected-pass tests. r=jrmuizel,kats
https://hg.mozilla.org/integration/autoland/rev/dccc79e2b399
Part4. Update reftest annotations for new failures. r=kats
Backed out for Windows bustage at gfx/layers/wr/StackingContextHelper.cpp(94):

https://hg.mozilla.org/integration/autoland/rev/e7da45f5f6a42f07cc00a94dc344babb63209dd6
https://hg.mozilla.org/integration/autoland/rev/3bc2bfadcf57de227142be7fa7d08ac87a5fd401
https://hg.mozilla.org/integration/autoland/rev/472e75fe095a87c35a3c6e29539cb446a630ad20
https://hg.mozilla.org/integration/autoland/rev/38bede6d40f6a9dba92ea3b0adba38fde47e5560

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=dccc79e2b399d5d2ff1a03d0f1bdb391b9f0a2bf&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=133819450&repo=autoland

13:44:16     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/vs2015u3/VC/bin/amd64_x86/cl.exe -FoUnified_cpp_gfx_layers11.obj -c -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DNDEBUG=1 -DTRIMMED=1 -DGOOGLE_PROTOBUF_NO_RTTI -DWIN32_LEAN_AND_MEAN -D_WIN32 -DWIN32 -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -DOS_WIN=1 -D_UNICODE -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DUNICODE -D_WINDOWS -D_SECURE_ATL -DCOMPILER_MSVC -DMOZ_ENABLE_D3D10_LAYER -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/gfx/layers -Iz:/build/build/src/obj-firefox/gfx/layers -Iz:/build/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -Iz:/build/build/src/ipc/chromium/src -Iz:/build/build/src/ipc/glue -Iz:/build/build/src/docshell/base -Iz:/build/build/src/layout/base -Iz:/build/build/src/layout/generic -Iz:/build/build/src/media/libyuv/libyuv/include -Iz:/build/build/src/gfx/skia -Iz:/build/build/src/gfx/skia/skia/include/config -Iz:/build/build/src/gfx/skia/skia/include/core -Iz:/build/build/src/gfx/skia/skia/include/gpu -Iz:/build/build/src/gfx/skia/skia/include/utils -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss   -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -deps.deps/Unified_cpp_gfx_layers11.obj.pp -utf-8 -TP -nologo -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -we4553 -GR-  -Z7 -O1 -Oi -Oy- -WX -Iz:/build/build/src/obj-firefox/dist/include/cairo    z:/build/build/src/obj-firefox/gfx/layers/Unified_cpp_gfx_layers11.cpp
13:44:16     INFO -  Unified_cpp_gfx_layers11.cpp
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(94): error C2065: 'Matrix': undeclared identifier
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(94): error C2146: syntax error: missing ';' before identifier 'transform2d'
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(94): error C2065: 'transform2d': undeclared identifier
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(95): error C2065: 'transform2d': undeclared identifier
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(96): error C2065: 'transform2d': undeclared identifier
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(96): error C2228: left of '.ScaleFactors' must have class/struct/union
13:44:16     INFO -  z:/build/build/src/gfx/layers/wr/StackingContextHelper.cpp(96): note: type is 'unknown-type'
13:44:16     INFO -  z:/build/build/src/config/rules.mk:1063: recipe for target 'Unified_cpp_gfx_layers11.obj' failed
13:44:16     INFO -  mozmake.EXE[5]: *** [Unified_cpp_gfx_layers11.obj] Error 2
Flags: needinfo?(bugmail)
Flags: needinfo?(bugmail) → needinfo?(ethlin)
Heads up for the relanding: it looks like the patches on this bug combined with the patches for bug 1359242 make layout/reftests/transform/animate-layer-scale-inherit-3.html pass again. The try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b98b16200dfa28057fd3214b192946049ffc9eb has this.

I plan to land bug 1359242 soon so you'll probably want to update your patches to mark that test passing.
Pushed by ethlin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0c8a91346605
Part1. Store inherited scale in stacking context. r=jrmuizel,kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed003bffcb51
Part2. Use stacking context's scale value to compute the correct paint rect. r=jrmuizel
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed4b70492285
Part3. Update reftest annotations for unexpected-pass tests. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e95ed440045
Part4. Update reftest annotations for new failures. r=kats
I fixed the compile error. Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac9f21bb4ca2a6dde20dc8f41402f9ede41ad898
Flags: needinfo?(ethlin)
Duplicate of this bug: 1402931
You need to log in before you can comment on or make changes to this bug.