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

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:

wrong

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

right

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:

tool
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:

wrong2

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

right2

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 17 (9 by maintainers)

Most upvoted comments

@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.

@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.

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.

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.

Duplicate of #

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

@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. 👍

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.

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.

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 😃

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 fair enough, thanks for taking the time to write such an informative reply. I’ll improve the issue report this afternoon (morning here now).

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.