[GLES2] Repeated textures look blurry in HTML5 builds

[theoratkin]

Godot version:
3.2.3.stable.official

OS/device including version:

The issue happens on HTML5 with GLES2 backend. Tested on Arch Linux with Firefox 83 and Chromium 87.0.4280.88.

Tested on two different computers, one has Intel i5-3320M and the other has NVIDIA GTX 1650 SUPER.

Issue description:

If you set "Repeat" to enabled in texture import settings, the texture will look blurry in HTML5 build, like it has been scaled with aliasing. This causes semitransparent edges and generally blurry look for pixel art.

The issue happens with GLES2 backend, it's not present on GLES3.

How it's supposed to look:

How it looks instead:

Steps to reproduce:

1. Add any pixel art texture to you project and make sure that you can see it up close.
2. Disable filtering and enable repeat in import settings and click "Reimport".
   
3. Export the project using HTML5 preset.
4. Run it in your browser. You will likely need to start a local HTTP server like this: python -m http.server --bind 127.0.0.1

Minimal reproduction project: blurred_repeated_pixels.zip

There's already an HTML5 build in the build/ directory.

[Calinou]

I can confirm this on Firefox 83 and Chromium 87 (Fedora 33, GeForce GTX 1080).

[clayjohn]

Can either of you try with high precision floats enabled?

https://docs.godotengine.org/en/stable/classes/class_projectsettings.html#class-projectsettings-property-rendering-gles2-compatibility-enable-high-float-android

[theoratkin]

@clayjohn Just tried it, it doesn't fix the issue.

[lawnjelly]

Always try this kind of thing with and without batching BTW as that can help pin it down, although in this case it is same with and without.

It is probably due to wrapping with a non power of two texture. Top tip, use POT textures on HTML5 / mobile, especially when using any kind of wrapping.

Yeah, it looks fixed if I change the texture to 512x128. It's a long standing aim to put warnings like this during the export process but no one has got round to it, so it keeps coming up! 😁

[theoratkin]

@lawnjelly Can confirm that using a power of two texture fixes the problem! Ironically, all textures in my project were power of two except for repeated ones. 🤦‍♂️

[theoratkin]

@larsonjj Is it a driver issue or can it be fixed within the engine? GLES3 doesn't have this problem.

[lawnjelly]

GLES3 mandates non POT texture support, GLES2 does not. Hence hacky workarounds are used on GLES2 which may look bad and run like a dog. But you can avoid this problem by just using POT textures.

[clayjohn]

I wonder if our POT codepath accidentally forces texture filtering.

@theogen-ratkin can you compare the texture displaying the issue with filter on and off?

[theoratkin]

@lawnjelly Oh I see. I agree that it's better to just adhere to the standard, but I think it would be nice if Godot had some fallback option. Maybe resize textures at export time?

@clayjohn No, with filtering the texture looks completely blurry. But you can see that the situation is the same: original image is consistent in color, the second one has semitransparent edges.

[lawnjelly]

@lawnjelly Oh I see. I agree that it's better to just adhere to the standard, but I think it would be nice if Godot had some fallback option. Maybe resize textures at export time?

What you are seeing is the fallback option 😁 . It does texture wrapping in the fragment shader instead of using hardware wrapping. While having an auto resize will lead to better performance (and I'm not against this), it could also trade other problems due to aliasing error between the old and new size (e.g. 104 pixels to 128, how do you resize to be pixel perfect?).

EDIT : Ignore me, totally wrong it is not using the shader here, it is actually doing the texture resize to power of 2 in this case, see later! 😊

[theoratkin]

What you are seeing is the fallback option

Fair enough! I just think that it can really hurt Godot's adoption, since people coming from, say, Unity, expect things to "just work" the same on all platforms, no matter what you throw at it. And even though there's Vulkan, GLES2 is not going away anytime soon.

Hence hacky workarounds are used on GLES2 which may look bad and run like a dog.

What kind of workarounds? Are they really so bad? If so, we could make those workarounds optional, so that developers could decide for themselves if they want them.

[clayjohn]

We do resize the texture at load time

godot/drivers/gles2/rasterizer_storage_gles2.cpp

Lines 677 to 686 in 6956b01

	if (texture->resize_to_po2) {
	if (p_image->is_compressed()) {
	ERR_PRINTS("Texture '" + texture->path + "' is required to be a power of 2 because it uses either mipmaps or repeat, so it was decompressed. This will hurt performance and memory usage.");
	}
	if (img == p_image) {
	img = img->duplicate();
	}
	img->resize_to_po2(false);
	}

I'm guessing that the blurriness is actually caused by the resize rather than the custom repeat shader code.

edit: Yep. The resize code using bilinear filtering by default and the resize_to_po2 code just uses the default

godot/core/image.h

Lines 228 to 229 in 6956b01

	void resize_to_po2(bool p_square = false);
	void resize(int p_width, int p_height, Interpolation p_interpolation = INTERPOLATE_BILINEAR);

This can be solved by passing in the textures filter setting to the resize_to_po2 code. We just need to add an optional boolean p_filter to resize_to_po2.

[theoratkin]

@clayjohn If that's the case then it's great! I thought that it looked like it just has been resized with filtering.

This can be solved by passing in the textures filter setting to the resize_to_po2 code. We just need to add an optional boolean p_filter to resize_to_po2.

I might do this and send a PR request, I wanted to contribute to Godot for some time now.

[lawnjelly]

I wonder if our POT codepath accidentally forces texture filtering.

This is possible. It sets the conditional CanvasShaderGLES2::USE_FORCE_REPEAT which may be causing a rebind and losing a filtering state. A graphics debugger would show this if we faked lack of non-pot support on desktop.

What kind of workarounds? Are they really so bad? If so, we could make those workarounds optional, so that developers could decide for themselves if they want them.

Wrapping in the fragment shader. Yes, depending on the hardware. You can't really make them optional, unless you mean crash, display garbage, or display a black texture or something, if you are asking the hardware to do something it can't do. A GPU isn't like a general purpose computer (at least not GLES2), it has a limited set of functions.

It's kind of like asking a car to fly. Maybe you'd seen Elon Musk with a flying car, but you want to try it on your car. It can't fly, so it fakes it by displaying some sky on the windscreen as a workaround. You're not satisfied with the workaround, you turn it off and insist on it flying, you drive it off a cliff, and that won't go so well. 😱

What you can do is display a warning to the user to tell them they are doing something that could be problematic. This hasn't been implemented. So you try and put your car in fly mode, and it bleeps at you and puts up a message saying WARNING! can't fly etc. 😄

[lawnjelly]

We do resize the texture at load time

Ah that does explain the blurryness. But then there is no need to do the wrapping in the fragment shader. So the current arrangement may not be good. Resizing the texture is preferable to the fragment shader approach imo.

I'm sure we have previously had bugs in the USE_FORCE_REPEAT conditional, so I do think it does use it (under certain circumstances at least). Maybe it is intended to use the texture resizing in some situations, and the shader in others. Or maybe the shader is a leftover? We should double check it is not doing the resize AND using the shader too.

EDIT : When I debugged the example it's doing the resize and I don't think it is using the wrapping shader, so that is good that it is not using both.

I checked up and #40373 and #40410 was the issue I was thinking of. From memory I think what can happen is the texture is imported as non wrapping, but this can be overridden in the texture rect. So it may be that as far as the texture storage is concerned, there is no need to resize to POT, but when it gets to actually drawing, it uses the shader wrapping (I haven't debugged this, but I now assume that is what is happening).

That does suggest we may not be able to get rid of the shader code after all, as it may be impossible to know ahead of time whether the texture will try to be drawn with wrapping. That's just my best guess of the existing arrangement, really to get it straight in my head 😀 as it turns out to be not straight forward which path is used.

[theoratkin]

We just need to add an optional boolean p_filter to resize_to_po2.

@clayjohn Wouldn't it be better to add a p_interpolation parameter like in resize() so that we could choose from all Interpolation members?

void resize_to_po2(bool p_square = false, Interpolation p_interpolation = INTERPOLATE_NEAREST);
void resize(int p_width, int p_height, Interpolation p_interpolation = INTERPOLATE_BILINEAR);

I tested resize_to_po2 with INTERPOLATE_NEAREST and it's noticeably better, almost perfect, but you can see that it's stretched vertically a bit (since the image is resized from 512x104 to 512x128). Maybe this could be fixed by adding empty spaces to image, so that its aspect ratio is preserved? EDIT: Though it wouldn't work, since it would introduce spaces on wrapping as well.

[clayjohn]

@theogen-ratkin

Wouldn't it be better to add a p_interpolation parameter like in resize() so that we could choose from all Interpolation members?

Yep. Good suggestion.

Also, when calling from rasterizer_storage don't forget that it will always have to be nearest. I was wrong to say you should switch between linear and nearest.

Regarding adding padding. I think stretching is a better result. Sure, it won't be good for pixel-perfect assets, but when using pixel perfect assets you need to be more careful designing them anyway. The majority of assets will be better off with a bit of stretching. And as you rightly point out, you would introduce space on wrapping and black bars.

[theoratkin]

@clayjohn

Yes, good point about stretching, I guess it should be up to developers to make sure that textures are sized properly.

I have just sent pull request #44445 with my changes to resize_to_po2().

[akien-mga]

Fixed by #44460.

[theoratkin]

Awesome! Thanks for letting me make my first contribution to Godot!