WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 136 forks source link

No antialias #234

Closed towolf closed 7 years ago

towolf commented 7 years ago

I've built the wpe buildroot and I cannot figure out why canvas or webgl are not antialiased.

Platform is a Raspberry Pi 2.

albertd commented 7 years ago

I expect this export is causing your non-antialias issue:

https://github.com/Metrological/buildroot-wpe/blob/master/package/wpe-launcher/wpe#L11

towolf commented 7 years ago

@albertd, nope, that's not it.

I unset this variable, then I set it to other values like msaa, mask and so on. And no change was seen.

And then I've even removed this patch and made a full rebuild and could see no change:

https://github.com/Metrological/buildroot-wpe/blob/master/package/cairo/0008-add-noaa-compositor.patch

Is there any other way to enable full anti-alias?

walmis commented 7 years ago

I've also stumbled upon this problem. Are there any updates on this?

Built master buildroot from WebPlatformForEmbedded (2017.03.30)

walmis commented 7 years ago

After some hacking and debugging, I've managed to isolate the offending line: https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/master/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp#L257 For some reason cairo antialiasing is explicitly disabled here. I've checked the upstream code, and it seems to be fixed there.

magomez commented 7 years ago

The RPi doesn't support MSAA, that's why antialiasing is disabled on purpose when performing gl drawings.

albertd commented 7 years ago

https://github.com/WebPlatformForEmbedded/WPEWebKit/blob/master/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp#L130 also includes this, @magomez maybe wise to guard them with the noaa environment flag?

magomez commented 7 years ago

Ok, I'll add it to my TODO list.

towolf commented 7 years ago

The RPi doesn't support MSAA, that's why antialiasing is disabled on purpose when performing gl drawings.

I'm not sure that is true. It can use MSAA and FSAA in other *GL uses.

Is this a specific reason, you are aware of, that MSAA cannot be used?

magomez commented 7 years ago

Last time I checked the video driver was not supporting the GL_EXT_multisampled_render_to_texture extension, so multisampling is not an option. FSAA could be used, but it requires much more video memory to perform the rendering, that's why it was discarded. I'll give a look to recent driver versions in case the msaa extension is now supported.

walmis commented 7 years ago

I'm running on i.MX6QP and there is support for GL_EXT_multisampled_render_to_texture (from weston startup output).

[08:16:23.513] GLSL version: OpenGL ES GLSL ES 3.00
[08:16:23.513] GL vendor: Vivante Corporation
[08:16:23.513] GL renderer: Vivante GC2000+
[08:16:23.513] GL extensions: GL_OES_vertex_type_10_10_10_2
               GL_OES_vertex_half_float GL_OES_element_index_uint
               GL_OES_mapbuffer GL_OES_vertex_array_object
               GL_OES_compressed_ETC1_RGB8_texture
               GL_OES_compressed_paletted_texture GL_OES_texture_npot
               GL_OES_rgb8_rgba8 GL_OES_depth_texture
               GL_OES_depth_texture_cube_map GL_OES_depth24 GL_OES_depth32
               GL_OES_packed_depth_stencil GL_OES_fbo_render_mipmap
               GL_OES_get_program_binary GL_OES_fragment_precision_high
               GL_OES_standard_derivatives GL_OES_EGL_image GL_OES_EGL_sync
               GL_OES_required_internalformat GL_OES_surfaceless_context
               GL_KHR_texture_compression_astc_ldr
               GL_EXT_texture_type_2_10_10_10_REV
               GL_EXT_texture_filter_anisotropic
               GL_EXT_texture_format_BGRA8888 GL_EXT_read_format_bgra
               GL_EXT_multi_draw_arrays GL_EXT_frag_depth
               GL_EXT_discard_framebuffer GL_EXT_blend_minmax
               GL_EXT_multisampled_render_to_texture GL_EXT_robustness
               GL_VIV_direct_texture
magomez commented 7 years ago

I'm working on this, but I haven't had luck finding a hardware to test my changes.

I'm gonna split this in 2 patches, one for the canvas and another one for webgl, cause the canvas one is for wpe only, while the webgl one should go upstream. @walmis, would you mind testing them?

If everything works, I'll just merge the canvas one and upload the webgl upstream (and then merge it in wpe as well if it can't wait until the next upstream merge)

towolf commented 7 years ago

So your patches are still not going to work with Rpi, right?

Personally, I would be fine if FSAA was an option. Using more memory is not a huge issue for me, the noaa however is a problem.

magomez commented 7 years ago

@towolf, nope, the patch won't work on the RPi. One way to get a better result with the canvas is by removing cairo_set_antialias(cr.get(), CAIRO_ANTIALIAS_NONE); in the ImageBufferData constructor and remove the CAIRO_GL_COMPOSITOR env variable (which is currently telling cairo to use the noaa compositor).

by doing this cairo will apply antialiasing, but it will do it on the CPU, which will damage the performance a bit, but maybe it's enough for your case.

towolf commented 7 years ago

Cool will try that. I don't need high framerates anyway.

walmis commented 7 years ago

@magomez Yes, I can test the patches, let me know what should be done.

walmis commented 7 years ago

And BTW, smaller canvases appear to be CPU rendered with cairo-image backend, while larger canvases are rendered using cairo-gl. How is antialiasing performed in those (CPU rendered) cases, for example if the platform does not have msaa extension.

magomez commented 7 years ago

Currently both small canvases (CPU rendered) and big ones (GPU rendered) don't use antialias at all due to the cairo_set_antialias(cr.get(), CAIRO_ANTIALIAS_NONE) in the ImageBufferata constructor. But I've changed this in the merge request I created. With it CPU rendered canvases will use default antialias, while GPU rendered ones won't use it at all if the gl extensions are not present, or use the default antialias value if the extensions are available.

If you could test https://github.com/WebPlatformForEmbedded/WPEWebKit/pull/244 to check that it works properly it would be awesome, @walmis

magomez commented 7 years ago

Nice, canvas changes merged. I'll work on another merge request to enable antialiasing on webgl as well.

towolf commented 7 years ago

Software antialias works on Rpi. Patched the source to make more flexible. If the ENV var is set we can disable the antialiasing. Why prevent it based on hard contraints. The user can test this and decide.

--- a/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp   2017-04-30 16:35:13.970725390 +0200
+++ b/Source/WebCore/platform/graphics/cairo/ImageBufferCairo.cpp   2017-04-30 16:39:46.909832548 +0200
@@ -70,14 +70,14 @@

 namespace WebCore {

-bool cairoIsUsingMSAA()
+bool cairoIsUsingNOAA()
 {
     // Cairo will use MSAA if the extensions GL_OES_packed_depth_stencil and GL_EXT_multisampled_render_to_texture
     // are available, and when it's told to use the msaa compositor through the CAIRO_GL_COMPOSITOR env variable.
     const char *env = getenv ("CAIRO_GL_COMPOSITOR");
-    bool usingMSAACompositor = env && strcmp(env, "msaa") == 0;
+    bool usingNOAACompositor = env && strcmp(env, "noaa") == 0;

-    return usingMSAACompositor && Extensions3DCache::singleton().GL_OES_packed_depth_stencil() && Extensions3DCache::singleton().GL_EXT_multisampled_render_to_texture();
+    return usingNOAACompositor;
 }

 ImageBufferData::ImageBufferData(const IntSize& size, RenderingMode renderingMode)
@@ -266,7 +266,7 @@
     RefPtr<cairo_t> cr = adoptRef(cairo_create(m_data.m_surface.get()));
     m_data.m_platformContext.setCr(cr.get());
     // Disable antialiasing if using the GL backend but cairo is not using MSAA.
-    if ((m_data.m_renderingMode == Accelerated) && !cairoIsUsingMSAA())
+    if (cairoIsUsingNOAA())
         cairo_set_antialias(cr.get(), CAIRO_ANTIALIAS_NONE);
     m_data.m_context = std::make_unique<GraphicsContext>(&m_data.m_platformContext);
     success = true;
magomez commented 7 years ago

Makes sense @towolf, I've commited that behaviour.

igor-borovkov commented 7 years ago

@wouterlucas @albertd @magomez this (the patch above) broke anti-aliasing on our boxes, we use NOAA explicitly, MSAA takes all GPU ram...

@vertexodessa @emutavchi

magomez commented 7 years ago

@igor-borovkov, can you elaborate, please? I don't get whether you were using antialiasing and it stopped working, or you weren't using it and now it's enabled. You say it broke, but then you say you're using NOAA, which means you're not using using it, so it cannot break.

Before this patch, no antialiasing was being used. With this patch, it's enabled by default, unless you set the env variable CAIRO_GL_COMPOSITOR="noaa".

igor-borovkov commented 7 years ago

@magomez @albertd @wouterlucas we've been using CAIRO_GL_COMPOSITOR="noaa" for a more than a year probably, probably since the time we started using WPE. Anti-aliasing (AA) was not working, until we switched to pull from a stable branch not far long ago! (still using NOAA!) AA was working since the switch.

Websites look like crap without AA honestly. But after we switched to a more recent revision on the stable branch very recently, AA stopped working (websites look like crap again). The upgrade included the change above which we are trying to workaround by reverting the patch locally.

We're still using CAIRO_GL_COMPOSITOR="noaa". We cannot use MSAA (we do have support for extensions I believe needed), but MSAA consumes a lot of GPU ram plus accelerated canvas is broken with MSAA (on my favorite website for canvas perf testing: http://www.smashcat.org/av/canvas_test/ a sphere is not rendered with MSAA). @emutavchi

UPDATE. I wonder what's the story behind NOAA compositor, is it unofficial? Is there any history left why Samsung made it ? And why it does AA when it's called NOAA and has checks to refuse working if AA is requested, so many things don't make sense so far.

vertexodessa commented 7 years ago

We're still using CAIRO_GL_COMPOSITOR="noaa". We cannot use MSAA (we do have support for extensions I believe needed), but MSAA consumes a lot of GPU ram plus accelerated canvas is broken with MSAA (on my favorite website for canvas perf testing: http://www.smashcat.org/av/canvas_test/ a sphere is not rendered with MSAA). @emutavchi

well actually I think there is a sphere on smashcat with MSAA until it crashes due to out-of-GPU-memory issue.. Update: the sphere is not rendered only when CAIRO_GL_COMPOSITOR is unset, i.e. the default span compositor is used

magomez commented 7 years ago

we've been using CAIRO_GL_COMPOSITOR="noaa" for a more than a year probably, probably since the time we started using WPE. Anti-aliasing (AA) was not working, until we switched to pull from a stable branch not far long ago! (still using NOAA!) AA was working since the switch.

Ok. Antialiasing was disabled for the canvases (both accelerated and not accelerated) since the beginning. It didn't really matter whether the noaa compositor was selected, AA was disabled due to performance reasons. I guess in your first pull from stable you picked 303ab48d6f9d7e7179ff37fe206c3ed50185af31, which was disabling AA only for accelerated canvases, but left it enabled for non accelerated ones.

Websites look like crap without AA honestly. But after we switched to a more recent revision on the stable branch very recently, AA stopped working (websites look like crap again). The upgrade included the change above which we are trying to workaround by reverting the patch locally.

I guess in this new upgrade you picked dffa050257a37beb53ed938b3d2a600adbf6699d, which again made the AA config the same for accelerated and non accelerated canvases, and made the non accelerated canvases not to use AA when the noaa compositor was selected. My guess is that you are only using non accelerated canvases, and that's why AA was disabled since always, then got enabled for a while and then disabled again. And what you want is to keep AA working on non accelerated canvases.

UPDATE. I wonder what's the story behind NOAA compositor, is it unofficial? Is there any history left why Samsung made it ? And why it does AA when it's called NOAA and has checks to refuse working if AA is requested, so many things don't make sense so far.

Ok, let me explain this a bit. The NOAA compositor is not official, and it was not made by Samsung, but by me (I didn't change the copyright in the patch). This compositor is basically a modification of the MSAA one. The goal of the MSAA compositor is to be able to perform opengl drawings with antialiasing but delegating all the antialiasing operations to the hardware. This means that cairo is not performing any software antialiasing while drawing, which improves the performance. But the MSAA compositor cannot be used if the required gl extensions are not available. You can consider the NOAA compositor as an MSAA without the hardware requirements. Cairo won't perform any antialiasing and delegate the drawing to opengl, which won't perform any antialiasing either, so the result won't be antialiased at all. This improves a lot the performance of the accelerated canvases when antialiasing is not required. And that's the reason why it checks that no AA should be required. When AA is required, or if the rendering is not accelerated, then it falls back to the spans compositor, so software antialiasing can be performed again.

Then, @igor-borovkov, if what you want is to have again antialiasing on non accelerated canvases, you can stop using the noaa compositor and that will make it. But it will also enable antialiasing on accelerated canvases (for coherence when rendering). If what you want is to keep AA on non accelerated canvases but keep it disabled when using accelerated ones, you can change the condition in ImageBufferCairo.cpp if (cairoIsUsingNOAA()) with if (cairoIsUsingNOAA() && m_data.m_renderingMode == Accelerated)

Hope my explanation is helpful :)

vertexodessa commented 7 years ago

hi Miguel, thank you for the explanation, it is very helpful. it looks like when antialiasing is enabled for accelerated canvases (with span compositor), sphere on http://www.smashcat.org/av/canvas_test/ is not shown this looks like a bug, but we have not tried on Raspberry PI and I'm not sure if it is reproducible there

magomez commented 7 years ago

it looks like when antialiasing is enabled for accelerated canvases (with span compositor), sphere on http://www.smashcat.org/av/canvas_test/ is not shown this looks like a bug, but we have not tried on Raspberry PI and I'm not sure if it is reproducible there

Curious, it works perfectly here on the rpi2. With a lower framerate, but the sphere is visible.