Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web]:Canvaskit ClipRect not being clipped properly #58547

Closed
mariamhas opened this issue Jun 3, 2020 · 18 comments
Closed

[web]:Canvaskit ClipRect not being clipped properly #58547

mariamhas opened this issue Jun 3, 2020 · 18 comments
Labels
assigned for triage issue is assigned to a domain expert for further triage c: rendering UI glitches reported at the engine/skia rendering level dependency: skia Skia team may need to help us e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web e: web_html HTML rendering backend for Web platform-android Android applications specifically platform-ios iOS applications specifically platform-linux Building on or for Linux specifically platform-mac Building on or for macOS specifically platform-web Web applications specifically platform-windows Building on or for Windows specifically

Comments

@mariamhas
Copy link

The Maskfilter.blur(BlurStyle.outer, 10) is doing its thing in a ClipRect with a default hard edge (I tried other edges too). Sometimes at some scales and resolutions on devices/desktop/canvasKit the outer blur that the ClipRect should cut off is bleeding out, causing a hairline artefact red line to appear that should not be there. I don’t know exactly why, but I suspect some kind of rounding issue of the clipping algorithm.

This is what it looks like at a screen/device scale situation when it appears:
unnamed

That hairline should never be visible if the ClipRect would correctly do what it is supposed to do. Alternatively it is the MaskFilter.blur(BlurStyle.outer, 10) that starts its outer blurring at some resolutions at the inner side already, preventing the ClipRect from cutting it away. Regardless of what is causing it, it should not happen and mostly it does not, but at some resolutions it does.

issue from @rydmike

@mariamhas mariamhas added platform-web Web applications specifically e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web labels Jun 3, 2020
@rydmike
Copy link
Contributor

rydmike commented Jun 3, 2020

Thanks @mariamhas for opening the ClipRect issue based just on the rough eMail discussion of it.
Below is a more detailed presentation of it with reproducible code example as well.


Example of ClipRect Issue

TLDR

ClipRect does not always clip correctly, there are often thin remnants left of what it should clip away.

Applies to

All Flutter versions (past ones too), all channels and all platforms.
The issue is not just an issue with CanvasKit, as far as I have noticed it applies to all platforms, using SKIA rendering as well as DomCanvas.

Steps to reproduce

This Gist: https://gist.github.com/rydmike/0c6a2412cb3222a02e25cfead9ba8d29 show an example of the ClipRect issue.

The example Gist can also be tested live in DartPad here:
https://dartpad.dartlang.org/0c6a2412cb3222a02e25cfead9ba8d29

Detailed Information

The above mentioned MaskFilter.blur(BlurStyle.outer, 10) case was where I first noticed it, while using it on CustomPaint objects to create a blur effect on a custom paint outer edges. This filtering also caused a blur on the paint container's (canvas) outer edges, that I then needed to cut away with with ClipRect. I then noticed that the blur effects outside the container did not always get clipped.

As shown in this code snippet, we can equally well demonstrate this by just putting a surrounding BoxShadow on a Container and try to clip it away with a ClipRect. The situation is the same as the more involved case where I noticed it. Normally one would probably not put an outer BoxShadow on a Container and the clip it away again directly with a ClipRect, seldom any point in doing that, but it is a simple way to demo the issue with ClipRect. As mentioned, a more real use case is when drawing something and using e.g. the outer blur style, or something else that would cause an effect outside the used container that you then want to clip away fully.

Example case

With the provided code snippet a Container with a BoxShadow is presented. When the switch is flipped, the same Container is wrapped with a ClipRect. At some Container sizes and/or media (window) sizes, we can observe very thin remnants of the box shadow effect on "random" edges, that ClipRect should have clipped away completely. The used clipBehavior does not have any major impact on the end result.

With this example the Slider can be used to interactively change the size of the Container and observe how the remnants appear and disappear at various edges as the size is changed. The same behavior can also be observed to some extent on Desktop or Web when changing window (media) size.

Screenshot examples

In DartPad (WEB), before turning ClipRect on:

image

In DartPad (WEB) after turning ClipRect on:

image

On a device, after turning ClipRect on:

image

On Windows Desktop, before turning ClipRect on:

image

On Windows Desktop, after turning ClipRect on:

image

@yjbanov
Copy link
Contributor

yjbanov commented Jun 4, 2020

@liyuqian You are our expert on anit-aliasing in the presence of clipping and such. Can you please help us triage this? Given the differences in behavior across all platforms, what do you think is the solution here? In fact, what are the requirements? How should the engine behave?

@yjbanov yjbanov added assigned for triage issue is assigned to a domain expert for further triage e: web_html HTML rendering backend for Web platform-android Android applications specifically platform-ios iOS applications specifically platform-linux Building on or for Linux specifically platform-mac Building on or for macOS specifically platform-windows Building on or for Windows specifically labels Jun 4, 2020
@rydmike
Copy link
Contributor

rydmike commented Jun 4, 2020

For the issue #58546 I made an example that also at the same time presents a use case where the ClipRect is not used to just clip away a box shadow that was put on it, but rather to cut away part of the filter effects that causes shadows to be painted also outside the container, that we then want/need to cut off for a real use case, but where the ClipRect sometimes fails.

The live version of this issue demo can be found here: https://rydmike.github.io/maskfilterskia/#/
The code in a Gist here: https://gist.github.com/rydmike/78cdff7f63515a9ea18f172b8640de55

The outer blur is here caused by Maskfliter.blur outer used on the custom paint wave painter.
image

We can try and see that also here turning on ClipRect fails to cut it away consistently, and the result varies with Container size. For some sizes it is gone completely, and for some sizes a remnant of it remains on one ore more sides randomly.

image

Findings summary

The issue and result above is of course the same as with the simple BoxShadow case. The ClipRect does not always completely cut off everything correctly that is drawn outside the container.

Alternatively shadows/blurs that should start on the outer side of the container at some sizes start their paint a bit inside the container. It smell a bit like some kind of rounding issue and/or consistency as to what is considered inside and outside of the container. Since the results vary with different container sizes and media sizes, it might also be rounding related from translation to device coordinates too. The difference could be as small as a >= versus > comparison at some critical rendering start/decision point.

@jmagman jmagman added this to Awaiting triage in Mobile - OS feature parity via automation Jun 4, 2020
@liyuqian liyuqian added the c: rendering UI glitches reported at the engine/skia rendering level label Jun 6, 2020
@liyuqian
Copy link
Contributor

liyuqian commented Jun 6, 2020

Yes, this is a fundamental issue for all anti-aliasing algorithms from Skia except for MSAA. So far the MSAA is only available on GPU for some limited operations, and the gradient/shadow is probably not one of them. The MSAA also has some big performance costs.

https://fiddle.skia.org/c/8c47cd24b73ac177d86b69a9c1376205 is a very simple Skia fiddle to illustrate the artifact without any Flutter code. The GPU rendering there probably has MSAA so the artifact disappeared. Chrome, Flutter, and Android all use Skia so it's unsurprising to see this artifact everywhere.

The easiest way to fix this is probably for app developers to remove the shadow/gradient in code using some if condition instead of relying on clip.

Flutter probably can't do anything to remove such artifact so I removed my assignment. CC @bsalomon @jvanverth for comments from Skia to see if Skia can fix this.

@liyuqian liyuqian removed their assignment Jun 6, 2020
@liyuqian liyuqian added the dependency: skia Skia team may need to help us label Jun 6, 2020
@rydmike
Copy link
Contributor

rydmike commented Jun 6, 2020

Thanks @liyuqian for looking into this. Already when I first saw this issue a year ago I suspected it was a rather low level issue (from Flutter's point of view), probably from the SKIA implementation.

I noticed in the Skia Fiddle that you made, that if I set AntiAlias to false, it works correctly, well at least for the sizes I tried with.

With regards to that, the Flutter ClipRect with the clipBehavior Clip.hardEdge is supposed to use just clipping without any anti-aliasing, yet it does not seem to be fully equivalent to using false in setAntiAlias(false) in your SKIA example. If it were, then using the hard edge clipping method in Flutter would work and ClipRect would do what it says on the tin for rectangles with this clipping method, but since it is not working, there is potentially also something else going on.


Issue Demo Case

Regarding the use case and suggested workaround, the first BoxShadow case was just a simple example I used to demonstrate the ClipRect issue, not a real use case that made any particular usage sense. Had it been an if condition would indeed have been a very simple workaround.

Real world use case

The real world use case, where I actually noticed the issue was much more involved and used a filter effect on CustomPaint, where the filter also has an effect outside the paint area, that we then wanted to clip away, while retaining the effect in the drawing area.

This screenshot series explain the real world case:
image

The code for the above demo can be found here:
https://gist.github.com/rydmike/78cdff7f63515a9ea18f172b8640de55
And you can play with it live, with a SKIA built version, here:
https://rydmike.github.io/maskfilterskia/#/

With the above live example you can use the slider to change the container's size and then observe how the ClipRect works OK at some sizes, while at other sizes a remnant on random edges of what it is supposed to clip a away, is left visible. It seems almost like some kind of rounding or precision issue with the clipping, regardless of where and why it is happening.

More on the real world use case

For the actually real world version, the waves also animate. Here is an example running on Windows at a resolution that happens to not show the issue:

WindowsVersion

And here a larger example recorded on a OnePlus7 Android device that just happens to have a combination of container height and media size width, where the ClipRect artifact thin line below the animated waves happens to occur and completely ruins (imo) the intended design:

OnePlusVersion

There are certainly a number of different things we can do to work around the issue here too, like putting it all in a Stack and drawing over the line that should never appear or maybe simpler just use an area for the waves that start at the bottom of the screen, like we already do on the left and right sides, that way the line would not really matter as it would be very hard to observe even if it is still drawn at the bottom of the display.

Since I expect that this ClipRect issue is not a thing that is going away very soon, I can and will certainly implement a work around for this particular use case too, that is not really a problem. I need to fix a minor responsive layout issue for this screen for really old small devices anyway, and can certainly work around this ClipRect issue at the same time.

ClipRect Issue Summary

My main red flag with the Flutter ClipRect class was that currently it is not fully capable of doing what its specification says that it should do. It sometimes leaves remnants of what it should clip away, but sometimes manages to do it correctly, the behavior is erratic and cannot be trusted.

The scenarios used to demonstrate this ClipRect issue and workaround to the demos or even real cases, to be able to live with the fact that ClipRect does not always produce specified results, are not really relevant to the issue.

@liyuqian
Copy link
Contributor

liyuqian commented Jun 6, 2020

@rydmike : the Clip.hardEdge only disables anti-alias for the clip. It does not disable anti-alias for other draw commands such as the drawRect in my Skia fiddle. What Clip.hardEdge does is more like this fiddle https://fiddle.skia.org/c/f30208f31d09421ab90dc96a15e6a97a where the clip is no longer anti-aliased but the rect is still anti-aliased, and the artifact retained.

@rydmike
Copy link
Contributor

rydmike commented Jun 6, 2020

@liyuqian Thanks for the info, it certainly explains the difference in behavior 😀


The actual Flutter issue remains though, ClipRect does not produce expected or even consistent results in Flutter. We get edge remnant artifacts sometimes, but not always and when we do get them, they may appear on different edges of the clipped rectangle! It does seem to depend on the rectangle size for where and when the they appear, so I guess that way it might be considered "consistent". 😏

@liyuqian
Copy link
Contributor

liyuqian commented Jun 6, 2020

@rydmike : as for your real use cases, maybe the TightClipper that I coded in this gist https://gist.github.com/liyuqian/65680f25573bb4339030ddecaf236232 could help. It's working on mobile, but not on DartPad/Web. @yjbanov : can you please check if Flutter for Web has some different handlings about CustomClipper?

I understand that ClipRect currently doesn't always render as expected due to the AA artifacts, and people can get frustrated about it. We definitely want to fix it. However, due to our limitations (the problem happens deep inside Skia, and removing the artifact from Skia may require a big performance hit), the TightClipper is the best solution that I can come up with so far. It's unfortunately not universal to all AA artifacts, and we have to devise specific workarounds for each case until we have a perfect AA algorithm one day.

@rydmike
Copy link
Contributor

rydmike commented Jun 6, 2020

@liyuqian Wow thanks!

I tried the TightClipper, works great for this particular use case and it worked on all platforms, also Web when you build for CanvasKit.

Sure it does not, like you already mentioned, work on Web when using DomCanvas, does not seem to do anything there, but it does not crash or cause any visible errors either, so that's good. This is a very reasonable and usable work-around for this particular use case, and probably in general for many other ClipRect issues too.

I was going to fiddle with the layout to hide the issue, but this is great, much simpler! 👍 😀

Is the usage of the TightClipper documented as a work-around to issues with ClipRect?
Could be a good note to show/mention in the docs, if it is not already there. I completely understand regarding the challenge and schedule of fixing the ClipRect and anti-alias artifact issue. To be honest it is not like it is a super big issue, for most use cases one might not even notice it, more of a nuisance in most cases, and for my particular case where it annoyed me the most, the TightClipper now does the job! 😀


Still now that I have seen it and know of the issue, I think I now notice it in some more places too. Like when you use Material, using anti-alias clip, that then also have custom shadow around the Material. Then the background in some cases bleed though the Material edge and its shadow, it should not be there, but sometimes it is, I'm guessing root cause is the same.

Looks like this when it happens. The left arrow points to a case where it shows, the right arrow to a case where it it does not show:

image


Sidenote

Of course the above design is ugly, it comes from a number of settings users can configure, the above config with colored dashboard items (depeding on module data source) using also a colored glow shadow behind the item, is the only selection/combo where it shows, it is not a particularly pretty combo, might even remove the possibility for the particular combo, by default it just looks like this:

image

Users can tweak the look a lot, a few more examples, just for fun! 😀

image

image

@DeadlyMissile
Copy link

@rydmike Can you share your color codes lol?

@rydmike
Copy link
Contributor

rydmike commented Jun 7, 2020

@DeadlyMissile Sure, but let's not get this discussion more off topic than I already made it above. You can direct message me on Twitter with the same handle as I use here on GitHub and we can discuss color value details. Of course, just using a color picker from screen shots of the above images will give you the color values too.

@liyuqian
Copy link
Contributor

@rydmike we've had some discussions regarding your question

Is the usage of the TightClipper documented as a work-around to issues with ClipRect?

We concluded that we should

  1. Document how AA works in our API docs and what "conflation" artifacts as experienced in this issue might be expected. Created Document how AA works in Flutter API docs and what "conflation" artifacts are expected #59701 to track this.
  2. We'll not document TightClipper hack in our API docs because it may bring more confusions than benefits as it's very case-specific. When new "conflation" cases arrive on Github, we'll reference them to the API doc on AA conflation, and provide specific advice on workarounds instead of giving a general "reduce clip by 1 device pixel" advice.

With that, shall we close this issue now?

@rydmike
Copy link
Contributor

rydmike commented Jun 18, 2020

@liyuqian I can resolve most of our issues now with the TightClipper "hack". So from that point of view our issues with the clipping are "resolved" and can be closed if so desired.

I do however want to point out that this clipping issue is apparent in many other situations than just the examples I made above. In my opinion the clipping issue should certainly also be of much more general interest and have some issue left open to track it and eventually resolve it, but sure not necessarily this case.

More clipping issue examples

This example shows a more common usage scenario and thus highlight the clipping issue point further.

Let's make a Material widget with rounded corners and maybe have a gradient inside it and use a custom BoxShadow to make a glow elevation effect "below" it, then also add an InkWell that should clip correctly to the rounded corners, maybe this widget is used as a value indicator or button of sorts. It could look something like this:

image

From the above screenshot we can already see that the white lines between the shadow and the rounded Material should NEVER be visible, yet they often are, it depends on the size of the widget and maybe also placement on screen. Our designers tells me "get rid of the white lines", but I cannot really do it, not easily at least. The clipping issue is there also if you just use normal Material elevation, but it is not as visible in that case due to the color used for the elevation, a glow shadow just makes it more visible.

I made a playground demo of this case too, the gist for it is here:
https://gist.github.com/rydmike/114c36d75084ab57716faea36993b8ce

A DartPad version can be played with here:
https://dartpad.dartlang.org/114c36d75084ab57716faea36993b8ce

And here is a ready compiled WEB SKIA CanvasKIT version of it:
https://rydmike.github.io/clipissue/#/

The rounded box version on top is just made with Material, with standard widgets the way one might compose such a widget. The one below it also wraps the Material with the custom TightClipper, but one that uses RRect in this case.

image

On the CanvasKIT version and devices, we can observe the thin white line come and go as we change the widget size, even on the one with a custom clipper, regardless of the tightFactor as well. Naturally on devices with higher device pixel ratio, the thin white line is very thin, so it is not so easy to see it, but on WEB CanvasKIT or standard Desktops with factor 1, it is very easy to observe it.

I get that since I just wrapped Material with the custom clipper and the Material is clipping before the custom clipper it is wrapped with, we can't get the desired correct clipping effect with this method for this case. Well we could, if we made our own Material version that uses a hacked version of ShapeBorderClipper, which on quick glance appears to be where the clipping happens for Material.

As for the DomCanvas WEB version on DartPad, for some reason the tight clipped Material version seems to work there! I cannot observe the thin white lines as soon as the tightFactor is close to 1 or higher, interesting!

image


The designers that have seen this issue once, just cannot stop looking at it and can observe it in many different places in many Flutter apps now and they do keep nagging about it saying it should not look like that. But yeah, it is what it is for now I tell them.

Here is another example, the white lines on the left and right sides should not be there and at some sizes they are not, which people notice as they resize the desktop window. The white lines are an artefact of this same clipping issue.

image

@liyuqian
Copy link
Contributor

Thanks for your new examples. To make it easier to track different kind of clip issues, I'm closing this one that's specific to TightClipper hack. Please feel free to open new issues that are not covered by TightClipper, and we'll find out solutions there.

Mobile - OS feature parity automation moved this from Awaiting triage to Engineer reviewed Jun 18, 2020
@rydmike
Copy link
Contributor

rydmike commented Jun 19, 2020

That is fine @liyuqian.
However, just to back-track a bit, this issue is actually already about clipping not working correctly in Flutter with SKIA in general. Still I agree, due to the mentioned TightClipper workaround for the case demonstrated by the reported specific use case affected by the general clipping issue, it has indeed become a bit unfocused, so a new issue focusing on the core clipping issue only, might indeed be more clear going forward.

If you are not already tracking the general clipping issue, discussed extensively in this issue and actually shown again above with another use case scenario, then I can certainly make a more focused new issue on demonstrating just the core problem. If you are already tracking it elsewhere, please let me know so I can follow it, and maybe if needed also add an example to it that can be used to demonstrate and repeat the issue.

@liyuqian
Copy link
Contributor

@rydmike : I'm not aware of any general AA clipping issue tracked on Github. I tend to not create meta issues that cover a lot of different things because they quickly become stale and non-actionable. I'll acknowledge and explain the general issue in the API doc (tracked in #59701), and use a Github issue to track each specific case that we can take actions. If there are many of such issues, we can create a label for those issues so we can have an overview of the progress through the issue list filtered by that label.

That said, please feel free to create a new issue for your core clipping case that's not fixed by TightClipper. I'll take a look at your code and investigate how to fix it as soon as I have some time.

@rydmike
Copy link
Contributor

rydmike commented Jun 28, 2020

@liyuqian, @yjbanov @mariamhas
The clipping issue, that we discussed in this issue extensively is very specific and @liyuqian you showed the underlying AA skia cause for it as well.

This clipping issue has a broad impact on many Widgets in Flutter, which is why I called it "general" above. All the issues reported here in this issue are specific examples that are all a consequence of the AA clipping issue. The end result is still that clipping in Flutter often causes incorrect results and it is not possible to predict when the result will be wrong.

That is why asked if this clipping issue is being tracked in the Flutter repo? It would be nice to have, just to be able to track it so we can see if/when it is solved. Therefore, I think it should be reported and open in Flutter too until it is solved.
Also other users that notice the issue could be referred to it.

Now that I know of it, I see impacts of it all over the place on Widget renders on any Flutter Skia platform, but the issues are more visible when device pixel ratio is 1, so it is often more obvious on desktop, but sure if you look closely on devices with 2...4 device pixel ratio, it is still visible on them as well.

The tight clipper was just one work around for one use case impacted by the AA clipping issue. Other examples cannot be fixed with the tight clipper work-around, eg the last example I posted. So maybe I should make a new issue for it then? My point was still though, that the root cause for the issue is the same AA clipping issue as the one already discussed here too, so it seems pointless to open another one, but I can certainly do so if that works better. I have a shorter simpler example I could use based on the last example I posted.

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
assigned for triage issue is assigned to a domain expert for further triage c: rendering UI glitches reported at the engine/skia rendering level dependency: skia Skia team may need to help us e: web_canvaskit CanvasKit (a.k.a. Skia-on-WebGL) rendering backend for Web e: web_html HTML rendering backend for Web platform-android Android applications specifically platform-ios iOS applications specifically platform-linux Building on or for Linux specifically platform-mac Building on or for macOS specifically platform-web Web applications specifically platform-windows Building on or for Windows specifically
Projects
Mobile - OS feature parity
  
Engineer reviewed
Development

No branches or pull requests

5 participants