SVG shape-rendering: crispEdges is ignored
Reported by
sky...@gmail.com,
Jun 15 2017
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.86 Safari/537.36 Example URL: https://codepen.io/sky87/pen/bRBqBB Steps to reproduce the problem: Apply "shape-rendering: crispEdges" to a vertical SVG line with an integer x coordinate What is the expected behavior? What went wrong? The line should not be antialiased. The rendering is correct in Firefox (compare the attached images). Does it occur on multiple sites: N/A Is it a problem with a plugin? N/A Did this work before? N/A Does this work in other browsers? Yes Chrome version: 59.0.3071.86 Channel: stable OS Version: 10.0 Flash Version:
,
Jun 16 2017
Adding RBS-label for now , as it seems to be recent regression. Please feel free to edit/remove this. Thanks!
,
Jun 16 2017
Typo error in comment #1, Good build: 60.0.3080.0 (Revision: 466837). Bad build: 60.0.3082.0 (Revision: 467534).
,
Jun 16 2017
This appears to be an issue with MSAA path rendering in Ganesh. Non-MSAA Ganesh produces the correct result. See attached image. My guess is that the tessellating path renderer is not snapping to full pixels when crisp edges is enabled, leading to antialiasing where there should be none (due to MSAA). senorblanco@, can you take a look?
,
Jun 16 2017
It doesn't look the the concave tessellating path renderer is used here, since the path is a 1px-wide line and convex (I enabled logging in the tessellator to verify). I don't see incorrect pixels on my Linux box with --enable-gpu-rasterization, but the line is offset by a pixel to the left. Brian, any thoughts? Seems like line drawing is doing something funky in MSAA mode.
,
Jun 16 2017
BTW, I don't think this should be a release blocker, since we've presumably lived with it on Mac and Android for a number of releases. Feel free to re-add if you disagree.
,
Jun 16 2017
Note that even on more complex content that does hit the path rasterization code, MSAA breaks crispEdges. See The example here: https://codepen.io/ericrk/pen/eRgmMr If you comment the "crispEdges" style in and out with MSAA supported, you should see no change (always smooth). If you do so with MSAA disabled, you see AA turn on and off.
,
Jun 16 2017
If this is not working across most of Skia (lines, paths), maybe we should just disable MSAA in CC in cases where any part of the doc requests no AA? Although it feels like Skia should support mixed AA/non-AA content when rendering to an MSAA backbuffer?
,
Jun 16 2017
Yikes, that's not good. We could disable MSAA if there's any crispEdges content in the page, although we'd have to snoop the SkPicture, most likely, and toggling modes might be jarring. Or maybe we can skip any path renderer that takes advantage of MSAA when AA is disabled, enforcing use of render in software and upload? That should fill all the samples with the same value, although I'm not 100% certain.
,
Jun 16 2017
Skia does support rendering aliased to a MSAA buffer when GL supports it. However, OpenGL ES doesn't support disabling multisampling rasterization without GL_EXT_multisample_compatibility. We could expose this through the command buffer.
,
Jun 16 2017
Actually, on Linux and Mac, I'm seeing aliasing on the tiger example, and MSAA is still enabled. If I comment out the crispEdges line, I see antialiasing again. So it looks like it's doing the right thing for concave paths. I think Skia is using glDisable(GL_MULTISAMPLE), which does the right thing even if the destination buffer is multisampled. So maybe there's just something wonky with lines.
,
Jun 16 2017
Or maybe ANGLE doesn't support glDisable(GL_MULTISAMPLE)?
,
Jun 16 2017
AGreed - it seems possible that ANGLE/DX may not be respecting glDisable(GL_MULTISAMPLE)... Adding jmadill@ in case he has some ideas. With --use-gl=desktop, the tiger example works fine (aliased with crisp edges). With ANGLE/DX, we end up seeing this issue (both in the line and tiger examples).
,
Jun 16 2017
actually add jmadill@
,
Jun 16 2017
So apparently ANGLE does not support the GL_EXT_multisample_compatibility extension on its D3D11 backend. However, it does support it on its GL backend, so the plumbing is there, and it should be fairly straightforward to implement (D3D11 has an equivalent state bit). However, one wrinkle is the SAMPLE_ALPHA_TO_ONE enum, for which Jamie wasn't sure if D3D had an equivalent.
,
Jun 16 2017
,
Jun 16 2017
Shannon, do you recall any details of our conversations about EXT_multisample_compatiblity on D3D11? I thought we talked about this with NVIDIA at some point - I don't know of a way to implement SAMPLE_ALPHA_TO_ONE trivially, without some shader rewriting. Maybe there's a blend state of some kind we could set to so the same operations, but my understanding of this feature and why it exists is a bit fuzzy. Otherwise ANGLE could expose a new extension that allows enabling/disabling multisample only - this is trivial in D3D11.
,
Jun 16 2017
For the purposes of this bug we'd be ok having a narrower extension that just did GL_MULTISAMPLE.
,
Jun 16 2017
I don't recall any discussions about EXT_multisample_compatibility-- I may not have been part of the conversations you're remembering.
,
Jun 20 2017
,
Jun 20 2017
I dug into this a bit today. At first I thought the SAMPLE_ALPHA_TO_ONE would be the part that would be difficult. The spec mentions how this could be emulated in D3D (maybe this is what Shannon contributed to the spec) and this would be possible in ANGLE. Turns out that the problems is that multisampling cannot be disabled in D3D11 feature level 10.1 and higher. From the docs: "For feature levels 10.1 and higher, the setting of MultisampleEnable has no effect on points and triangles with regard to MSAA and impacts only the selection of the line-rendering algorithm..." I made a test to confirm that this flag has no effect, the rendered results are always the same. Going to speak with our contacts at Microsoft today and see if there is something I'm missing.
,
Jun 21 2017
It sounds like D3D11 Doesn't support disabling multisampling on a per-draw-call basis. Given this I think our best option is to update the "content is suitable for GPU rasterization" detection to always return true (not blacklist to MSAA) if any content requires crisp edges. This may require an additional bool (requires_crisp_edges?) and detection code. The root of the logic is here: https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host.cc?rcl=b01dc5448d235367e67b6bc4c1e58aa54172fa97&l=736 Adding some additional people, in case anyone wants to take a look.
,
Jun 21 2017
Taking a look at this.
,
Jun 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/52796123f44bf2407ba058e77159156f6da3824d commit 52796123f44bf2407ba058e77159156f6da3824d Author: Eric Karl <ericrk@chromium.org> Date: Wed Jun 28 23:33:14 2017 Handle MSAA + Non-AA paths on Windows ANGLE doesn't support disabling MSAA on a per-draw-op basis, so we need to track whether *any* path needs MSAA disabled, and disable MSAA in general. To achieve this, this CL includes two changes: Addition of HasNonAAPaint() to PaintOpBuffer - this allows us to propogate the presece of any draw commands requesting non-AA raster through to the layer tree host impl. Renaming and inverting SuitableForGpuRasterization to HasSlowPaths. This updates the naming to represent what's actually happening, as we now always use GPU raster, even when "not suitable". Bug: 733691 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I3c2b6b55f3ecabcdc6be6312260c4bb9b2fc9ed6 Reviewed-on: https://chromium-review.googlesource.com/549236 Commit-Queue: Eric Karl <ericrk@chromium.org> Reviewed-by: Zhenyao Mo <zmo@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Reviewed-by: Stephen White <senorblanco@chromium.org> Reviewed-by: enne <enne@chromium.org> Cr-Commit-Position: refs/heads/master@{#483193} [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/layer.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/layer.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/picture_layer.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/picture_layer.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/picture_layer_impl_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/picture_layer_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/layers/texture_layer_impl_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/paint/display_item_list.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/paint/paint_op_buffer.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/paint/paint_op_buffer.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/paint/paint_op_buffer_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/test/fake_content_layer_client.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/test/fake_content_layer_client.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/test/fake_picture_layer.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/test/fake_picture_layer.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/test/test_web_graphics_context_3d.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host_impl.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host_impl.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host_impl_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/cc/trees/layer_tree_host_unittest.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/gpu/command_buffer/common/capabilities.h [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/gpu/command_buffer/service/gles2_cmd_decoder.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/gpu/command_buffer/service/gles2_cmd_decoder_passthrough.cc [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/tools/metrics/histograms/enums.xml [modify] https://crrev.com/52796123f44bf2407ba058e77159156f6da3824d/tools/metrics/histograms/histograms.xml
,
Aug 10 2017
|
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by hdodda@chromium.org
, Jun 16 2017Components: Internals>GPU>Rasterization
Labels: hasbisect-per-revision M-60
Owner: ericrk@chromium.org
Status: Assigned (was: Unconfirmed)