godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.23k stars 19.18k forks source link

2D Camera viewport rounding error with subpixel position of 0.5 with "Snap 2D Transforms to Pixel" enabled #93712

Open alvinhochun opened 2 days ago

alvinhochun commented 2 days ago

Tested versions

System information

Godot v4.3.beta2 - Windows 10.0.19045

Issue description

With rendering/2d/snap/snap_2d_transforms_to_pixel enabled, a 2D camera following an object positioned at a subpixel value of 0.5 when position is between 0 and half viewport size.

Steps to reproduce

Screenshots ![screenshot 1](https://github.com/godotengine/godot/assets/2397650/10f2ac81-2b5f-4969-96f1-f4d3283b5772) ![screenshot 2](https://github.com/godotengine/godot/assets/2397650/e5a9870a-e8aa-4bad-86fc-dbef8aab3f99) ![screenshot 3](https://github.com/godotengine/godot/assets/2397650/cd621a93-eed5-4c8e-8e38-6d132f4a62ff)

Minimal reproduction project (MRP)

camera-pixel-rounding-bug.zip

markdibarry commented 2 days ago

@alvinhochun That's good call out! Let me make the change real quick and see if it fixes your issue.

Edit: Yep! Seems to fix the issue. Though maybe we should make an MRP with a scene that better shows why this is an issue. image

alvinhochun commented 2 days ago

I'm not sure I get what you mean? The correct behaviour should be that the red and green squares always completely overlap with each other. Your screenshot is even more broken (on beta2 there is no offset at x > 80).

markdibarry commented 2 days ago

Oh interesting. With your suggested change of floor(x + 0.5), all 0.5 values show up with the red and green.

I was saying this MRP is good for measuring the issue, but I personally would like to see an example of a scene where this is obviously problematic behavior.

I think this stems more from the fact that the camera's position isn't automatically rounded as well. Up until now, I've just always just had my camera scripts have a global_position = target.global_position.round(), which does fix the discrepancy. The floor(x + 0.5) may be the way to go in the long-run if only to fix the shift between positive and negative, but probably unrelated to this issue. We may want to consider making the camera round when it renders as part of the pixel snap setting or something separate.

alvinhochun commented 2 days ago

Yeah, my comment over at https://github.com/godotengine/godot/pull/87297#issuecomment-2197619761 stemmed from me checking this issue. While these two issues are linked in some way (changing the pixel snapping may affect how the camera is or should be rounded) the root causes are likely not the same.

What puzzled me with this issue is how the rounding difference only exists between 0 and half viewport size on each axis. It stops appearing at x > 80 and y > 60 for the viewport size 160 x 120, and doesn't happen with any negative coordinates, which doesn't quite make sense to me yet.

markdibarry commented 2 days ago

IIRC, this was planned as a two-step change, since the camera has never floored/rounded its position, but Camera2D is much much more complex to alter (if anyone wants to step up, please be my guest!). In the meantime, the line in my previous comment will take care of any inconsistencies/jitters, where prior to https://github.com/godotengine/godot/pull/87297 there was no workaround. Again, I think the change from rounding to floor(x + 0.5) is the right call, but I don't think we'd see any benefits until the camera changes are implemented. I plan to take another crack at it, but I'd prefer if anyone more familiar with the Camera2D -> renderer code could weigh in!

alvinhochun commented 1 day ago

Hmm, looking at it I don't know how the camera should even be rounded if we want Godot to do it automatically. What happens if the camera is zoomed in/out? What if the offset is not integral? What if it is rotated?

Okay, let's just say hypothetically we somehow determine the camera should snap to pixel, should it be done based on the top-left corner of the final viewport rect, or the position of the Node2D?

Speaking of position, do you know that Camera2D.global_position = target.global_position.round() or Camera2D.global_position = (target.global_position + Vector2(0.5, 0.5)).floor() will actually position the camera incorrectly if the target's chain of parents contain nodes with certain transforms?

You can try this: Add a Node2D and set its X to 0.6, then add a Sprite (or a Polygon) as a child and set its X also to 0.6. It's global X position should be 1.2, which would normally round to 1, but you can see it's actually snapped to X=2. This is because each canvas item snaps to pixels relative to its parent, not globally. The parent Node2D is snapped to X=1, the child is snapped to X=1, so its global position after snapping is 1 + 1 = 2. So, to calculate the accurate camera position from a node, one needs to walk the parent chain, round the local position one by one, adjust for any transforms and accumulate the result.

Oops, that was a bit random.

alvinhochun commented 1 day ago

After a bit more digging, I think I understand the cause of this rounding error (disregarding rounding of the camera itself). It has to do with the top-level viewport transform.

Consider the case of the red square and camera being at x=0.5:

So, I believe the fix is to readd the viewport transform rounding removed in #87297, but using ceil(x - 0.5) instead. I'll make the PR in a bit. (Edit: This fails to account for odd viewport sizes... will need to do a bit more.)

I don't know if we should still consider rounding the camera, since it appears to be more involved per my previous comment. Maybe we should track that in yet another separate issue?