1395501 - SVG content has wrong resolution in layers-free mode

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P1)

Description

[Ethan Lin[:ethlin]]

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".

Comment 1

[Ethan Lin[:ethlin]]

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.

Comment 2

[Ethan Lin[:ethlin]]

Attachment: Bug 1395501 - Part1. Store inherited scale in stacking context.

Review commit: https://reviewboard.mozilla.org/r/176764/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/176764/

Comment 3

[Ethan Lin[:ethlin]]

The patch works but I need to fix the offset problem after scaling..

Comment 4

[Ethan Lin[:ethlin]]

Attachment: Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

Review commit: https://reviewboard.mozilla.org/r/177558/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177558/

Comment 5

[Ethan Lin[:ethlin]]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176764/diff/1-2/

Comment 6

[Ethan Lin[:ethlin]]

Attachment: Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

Review commit: https://reviewboard.mozilla.org/r/177560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/177560/

Comment 7

[Ethan Lin[:ethlin]]

Comment on attachment 8905754 [details]
Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177558/diff/1-2/

Comment 8

[Ethan Lin[:ethlin]]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177560/diff/1-2/

Comment 9

[Jeff Muizelaar [:jrmuizel]]

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?

Comment 10

[Ethan Lin[: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

Comment 11

[Ethan Lin[:ethlin]]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176764/diff/2-3/

Comment 12

[Ethan Lin[:ethlin]]

Comment on attachment 8905754 [details]
Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177558/diff/2-3/

Comment 13

[Ethan Lin[:ethlin]]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177560/diff/2-3/

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

https://reviewboard.mozilla.org/r/176764/#review186864

Comment 15

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

https://reviewboard.mozilla.org/r/177560/#review186866

Comment 16

[Jeff Muizelaar [:jrmuizel]]

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

Comment 17

[Ethan Lin[:ethlin]]

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

Comment 18

[Kartikaya Gupta (email:[e-mail])]

You shouldn't need part 3 anymore either since layers-free is on by default now.

Comment 19

[Ethan Lin[:ethlin]]

Attachment: Bug 1395501 - Part4. Update reftest annotations for new failures.

Review commit: https://reviewboard.mozilla.org/r/184568/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/184568/

Comment 20

[Ethan Lin[:ethlin]]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176764/diff/3-4/

Comment 21

[Ethan Lin[:ethlin]]

Comment on attachment 8905754 [details]
Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177558/diff/3-4/

Comment 22

[Ethan Lin[:ethlin]]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177560/diff/3-4/

Comment 23

[Ethan Lin[:ethlin]]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/176764/diff/4-5/

Comment 24

[Ethan Lin[:ethlin]]

Comment on attachment 8905754 [details]
Bug 1395501 - Part2. Use stacking context's scale value to compute the correct paint rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177558/diff/4-5/

Comment 25

[Ethan Lin[:ethlin]]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/177560/diff/4-5/

Comment 26

[Ethan Lin[:ethlin]]

Comment on attachment 8913143 [details]
Bug 1395501 - Part4. Update reftest annotations for new failures.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/184568/diff/1-2/

Comment 27

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8904954 [details]
Bug 1395501 - Part1. Store inherited scale in stacking context.

https://reviewboard.mozilla.org/r/176764/#review189738

Comment 28

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8905759 [details]
Bug 1395501 - Part3. Update reftest annotations for unexpected-pass tests.

https://reviewboard.mozilla.org/r/177560/#review189740

Comment 29

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8913143 [details]
Bug 1395501 - Part4. Update reftest annotations for new failures.

https://reviewboard.mozilla.org/r/184568/#review189742

Comment 30

[Pulsebot]

Pushed by [e-mail]:
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

Comment 31

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 32

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
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

Comment 34

[Ethan Lin[:ethlin]]

I fixed the compile error. Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac9f21bb4ca2a6dde20dc8f41402f9ede41ad898

Comment 35

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/0c8a91346605
https://hg.mozilla.org/mozilla-central/rev/ed003bffcb51
https://hg.mozilla.org/mozilla-central/rev/ed4b70492285
https://hg.mozilla.org/mozilla-central/rev/2e95ed440045