Godot: Treating one pixel width lines differently gives wrong results when the canvas is scaled.

[m6502]

Godot version:

3.2.2

OS/device including version:

Windows, GLES2, GLES3.

Issue description:

Drawing 1 pixel thick lines gives wrong results both in tool mode in the editor and when in game. They are not scaled accordingly with the 2D view, they are literally 1 "native resolution" pixel thick because an exception is made when the thickness is <= 1.0 and only two vertices are used instead of four. If line thickness is set to 1.001, for example, then it works as expected.

Current behavior:

Expected behavior (after modifying the line drawing code so as to not make an exception):

I'm attaching a minimal project showing the problem.

line_width_1.zip

This project draws 7 vertical lines using widths 0.12, 0.25, 0.50, 1.0, 1.01, 2.0 and 3.0:

extends Node2D

func _draw():

    draw_line(Vector2(10, 0), Vector2(10, 1000), Color.white, 0.12, false) ;
    draw_line(Vector2(15, 0), Vector2(15, 1000), Color.white, 0.25, false) ;
    draw_line(Vector2(20, 0), Vector2(20, 1000), Color.white, 0.50, false) ;
    draw_line(Vector2(25, 0), Vector2(25, 1000), Color.white, 1.00, false) ;
    draw_line(Vector2(30, 0), Vector2(30, 1000), Color.white, 1.01, false) ;
    draw_line(Vector2(35, 0), Vector2(35, 1000), Color.white, 2.00, false) ;
    draw_line(Vector2(40, 0), Vector2(40, 1000), Color.white, 3.00, false) ;

Current behavior:

Expected behavior (after modifying the line drawing code so as to not make an exception):

[Calinou]

I'm fairly sure this was done by design so you can work with more precision at high zoom levels.

Also, notice how lines are overlapping on the second screenshot. This needs to be fixed if we decide to do that route.

[m6502]

I think it makes sense to have "high precision" lines and that's why I didn't want to remove the code, but place it inside the preprocessor directives. Anyway the behavior seems unexpected to me. It's basically selectively ignoring the requested width. Wanting to draw 1 pixel width lines that are half the thickness of two pixel width lines seems to be the right thing to me.

About the overlap I was going to say that it was expected as the origin of some lines and the destination of the other are the same and I'm drawing with alpha, but in reality that's a draw_rect() call and not four draw_line() calls, so in fact lines shouldn't overlap and that is effectively another issue :-) I may try to give it a spin too.

[pouleyKetchoupp]

@m6502 Your contribution to the project is welcome, but let's go step by step through the proper process to make sure we find a solution that's compatible with the way the engine and editor currently work.

About the issue itself:
Please add a short description of your use case so others will have a better idea about what you're trying to achieve, and if it's editor-specific or when the game is played too.
Also, a minimal project would help a lot too for quickly testing your use case.
This will help figuring out if a simple script solution exists for this problem.

About the PRs:
Changes with preprocessor directives are probably not the best approach, as most users don't build their custom version. In case there's a need for an option in the engine, project settings are usually the best direction:
https://docs.godotengine.org/en/stable/classes/class_projectsettings.html

These PRs are not valid in their current state, so I think you can close them for now until there's a better assessment about the proper solution.

If you intend to share some code you've made for testing purpose, you can just link your commit directly like this:
https://github.com/godotengine/godot/pull/41177/commits/4a2c8e3bc29ed1f7abc88fcaaf8f4fe40617f2d6

Here are some general rules about how to setup and format PRs properly:

• The commit message, PR title and PR description need to specifically explain what the changes in code are.
• You need to make sure you target the proper branch for the PR to work properly. If you see too many file changes while you're setting it up it means you're targeting the wrong one.
• You can setup your PR as draft when it's still work in progress so it doesn't trigger reviews automatically.

[m6502]

@pouleyKetchoupp fair enough, thanks for taking the time to write such an informative reply. I'll improve the issue report this afternoon (morning here now).

[m6502]

OK, I edited the issue to include a small test project, along with the code as a snippet for convenience, and two screenshots showing before and after applying the patch.

[pouleyKetchoupp]

@clayjohn @lawnjelly Would it make sense to add an option in project settings or an extra argument to draw_line in order to allow sub-pixel line width? I don't see any easy workaround for this issue otherwise.

[m6502]

The issue is not really about sub-pixel but about zooming the view. A sub-pixel line may be many pixels thick if you get close enough.

Maybe the parameter should be about using world or screen space width. Or have a separate draw function. I clearly see the need for being able to draw in the two ways.

[lawnjelly]

I agree with @Calinou I see it as an established convention. Perhaps not ideal in retrospect, but changing it would lead to compatibility breakage. Certainly changing it for 3.x would be a controversial idea.

It is not super obvious, but the reason for the difference is sound - there is a hardware accelerated path for single pixel lines, but not necessarily for wide lines.

It could potentially be changed for 4.x, having e.g. a different function that guarantees a single pixel line that could be used from e.g. the IDE code no matter what the scaling, and then changing the existing one to switch to a scaling solution if the scale was not 1. This would be considerably more involved than the PRs so far though, involving a new render types, functions etc. Or there could be a flag on the line rendertype that indicated whether to scale 1 pixel width lines.

Personally I'm not convinced it will be worth it in terms of the extra code and bug surface, versus the existing quirk of setting line width to e.g. 1.01 if you want a scaling line. But it is doable. Someone who wanted to change should probably ask @reduz his opinion before doing any changes in this area, because he might be happy with the situation as is.

Another simpler alternative is to hard code e.g. gdscript etc to override and use 1.01 width when you try to select 1 width (although this would be horribly slow), then add another function to the language APIs to hard code single pixel width line. There might be other ways of doing it, these are just top of my head.

Or we could simply put a note in the docs that if you want a scaling line, to use 1.01 width.

We could also potentially change the exception rule in 4.x to be 1 pixel width + or - some small amount, to allow smaller lines when scaled (or some variation on this).

[m6502]

Maybe a negative size (and a constant with a meaningful name) could be a pixel perfect line. Easier to change, and more logical than any arbitrary minimum size :-)

[pouleyKetchoupp]

@lawnjelly As mentioned by @m6502 the current use case is more a problem of drawing lines with width <=1 on a zoomed canvas, where the final rendering would normally be actually more than one pixel. You can also reproduce it when running the game. With a scale of 10 on the canvas, it currently draws a line of width 0.5 as a 1 pixel line, rather than 5 pixels, but it draws a line of width 2 with 20 pixels.

[lawnjelly]

Maybe a negative size (and a constant with a meaningful name) could be a pixel perfect line?

Yes potentially, that is a good idea .. I'm not familiar with the API side (only the bit in the render backend). Setting the default to this. It still has a small risk, because anyone specifying a width of 1 will get drastically lower performance.

[lawnjelly]

@lawnjelly As mentioned by @m6502 the current use case is more a problem of drawing lines with width <=1 on a zoomed canvas, where the final rendering would normally be actually more than one pixel. You can also reproduce it when running the game. With a scale of 10 on the canvas, it currently draws a line of width 0.5 as a 1 pixel line, rather than 5 pixels, but it draws a line of width 2 with 20 pixels.

Sorry, I only edited my post to add this shortly before you replied. Agree that this could be better.

Also a note on PRs - we haven't ported GLES2 and GLES3 to 4.x yet, so it is likely anything in the 4.x branch could get overwritten (especially with batching lines), so it might be wise to hold off on 4.x specific changes in the backends (except in vulkan), although we can certainly come up with solutions ahead of time. :+1:

Maybe it should go in godot proposals? With pros and cons of different approaches so juan can have a look, as he will probably make the final decision on this as it will affect vulkan too.

[pouleyKetchoupp]

Maybe it should go in godot proposals? With pros and cons of different approaches so juan can have a look, as he will probably make the final decision on this as it will affect vulkan too.

Agreed, this discussion seems more involved than just deciding of a fix or a small change in code.
@m6502 Please open a ticket in there and follow the template:
https://github.com/godotengine/godot-proposals

Also for info:
In 4.0, the 1-pixel rule is handled in the server directly, so it seems that it would be easier to allow scaled lines with width <= 1, by adding an extra argument or handling negative values in a special way:
https://github.com/godotengine/godot/blob/d3b5c0948c943b3fbcb4b5322262c59c92abfa83/servers/rendering/rendering_server_canvas.cpp#L465-L487

[mc4man]

Duplicate of #

-

[m6502]

Duplicate of #

Really? I searched for it before opening the issue. In any case I'll open the proposal as I think it's worth it.

[pouleyKetchoupp]

Looks like it might be this issue: #19844

But yeah a proposal would be better for this discussion anyway, and I don't see any on the topic.

[m6502]

Not sure how could I overlook that! Maybe I did the search with the draw_rect() function instead, as that's how I found the issue. Well then, closing this issue and let's continue later when I write the proposal, thanks everyone for the comments.