Sprite atlas bleeding with blade renderer

[jansol]

Check for existing issues

• Completed

Describe the bug / provide steps to reproduce it

Sporadically appears and disappears. May appear in one window but not another within the same Zed instance. Similar to #643

Environment

Zed: v1.0.0 (Zed Dev e19339b)
OS: Linux 1.0.0
Memory: 30.9 GiB
Architecture: x86_64

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

[apricotbucket28]

I think this may be related to #11056 (probably the same cause), though the issues described are a bit different.

This is one of the more extreme cases I got while using Zed:

It's also visible with the LSP loading indicator:

[sk337]

i am having this issue as well

[apricotbucket28]

Hey @kvark, sorry to bother you! I'm not experienced with shaders, but this specific issue bothers me quite a lot. Do you have any guesses as to what could be causing this? e.g. could this be an issue with a specific primitive shader?

FWIW this may be related to #12599

[kvark]

I'm not able to see it on my platform. It would help to get a GPU capture with RenderDoc and share it somewhere.

[apricotbucket28]

Thanks for the reply. I've used RenderDoc to generate two different captures (one at 100% scaling and another at 125%, where the issue is clearer).
bleeding.zip

[kvark]

That's very helpful! Unfortunately, RenderDoc doesn't properly associate resources with the draw calls for this case, so I'm not able to debug it in the capture:

Based on the captured data though, we are drawing right on the edge of pixel centers. This isn't expected. This logic is exactly matching between the original Metal backend and Blade, though. So my leading hypothesis is that the issue is in gpui itself and is platform-agnostic. The reason why we only see it on Linux is probably due to how the default texture contents (for the atlas) are not initialized to zero and happen to have opaque garbage regions:

I'll keep looking at this, but I expect the solution to be outside of Blade-specific code.

[kvark]

Awktually, a better idea - it's caused by incomplete handling (in gpui) of the fractional scaling. MacOS users/devices tend to not use/enable it as much, I suppose, hence the issue wasn't detected earlier.

[jansol]

But it also happens when display scale is 100%? My initial guess was that vulkan and metal sample textures differently (0,0 at the corner of the pixel vs in the center of it) but I haven't had time to dig into whether that is the case.

[kvark]

No, the pixel center logic is exactly the same between them, this shouldn't be a difference here (for context, the only API where it's different is DX9). Do you have a RenderDoc capture with 100% scaling by any chance? My expectation is that all coordinates that the vertex shaders operate on would be exactly on the pixel boundaries in this case.

For example, in "bleeding_125scaling" capture that @apricotbucket28 provided, I see Y vertex output absolute coordinates being 789.499035 to 771.998565. Clearly, the left boundary is problematic here.

The exact data is coming from MonochromeSprite::bounds. One suspect place is Window::paint_glyph (or at least one of the places where the bounds are constructed), but it doesn't quite look like it could produce fractional coordinates.

[kvark]

I think I see what's happening in there. The layout is done in the coordinate system that doesn't match the screen. All coordinates get multiplied by scale_factor before going to the screen:

let scale_factor = self.scale_factor();
let bounds = bounds.scale(scale_factor);

In the content coordinates, we have primitives that are placed on the edge of the pixels. After multiplication, we get coordinates that end with .0, .25, .5, and .75.

Before these go into the gl_Position output, they also get divided by the screen size, and then the fixed hardware multiplies them back by the same size before rasterization. So this divide+multiply by a number (say, 897 in one of the captures) is pushing the ".5" resulting value slightly away from ".5". It could be ".4999034" for example, or ".500032". The difference is important though since it would determine if the pixel center is included in rasterization. We could address this by adding a small delta to the pixel coordinates if we know ".5" is our subpixel offset.

But that's only half of the problem. Even if ".5" was rasterized consistently, we'd still have another problem: if the pixel center happens to be too close to the primitive boundary, then the interpolated texture coordinate at this pixel will be too close to the atlas region boundary. Therefore, sampling this texel linearly will include the neighbors.
There are at least 2 ways to address this:

1. Include a black transparent border (of 1 texel) around every atlas region. It costs in texture space and the allocator complexity.
2. clamp the texture coordinates in the fragment shader, so that they are within half-texel from the boundary. That's what we did in WebRender. It costs in a bit of FS performance.

[kvark]

More concretely, here is a solution I'm thinking:

1. provide all coordinates into the shader in the content space (even u16 if possible, so that it's small).
2. provide scale_factor separately in the global uniforms
3. let the vertex shader figure out how to map this primitive onto the pixel grid, and also adjust the interpolated values like texture coordinates for consistency
4. let the fragment shader clamp the interpolated texture coordinates to the inner area of the atlas region

[jansol]

Sounds like that might actually fix the squiggly lines changing with display scale too!

[apricotbucket28]

Thanks for looking into this! My knowledge with graphics is little to none, and what you're describing seems like a much better solution.
Doing that might actually solve a lot of graphical issues as well, e.g. #7436 (like @jansol mentioned) or other cases of bleeding that are still left (like the following)

[JosephTLyons]

This is now included in today's v0.148.0-pre release, and will go out to stable next week.