Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
alvinhochun opened this issue Jun 28, 2024 · 8 comments · Fixed by #93786

Comments

@alvinhochun
Copy link
Contributor

alvinhochun commented Jun 28, 2024

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

  • Run MRP
  • Press Shift + Right until x is 0.5
  • Observe the offset between the red and green squares
    • The red square is being moved, which the camera follows
    • The green square is fixed to screen
  • Hold Ctrl + Right until x is larger than 80
    • Observe the lack of offset between the squares
Screenshots

screenshot 1
screenshot 2
screenshot 3

Minimal reproduction project (MRP)

camera-pixel-rounding-bug.zip

@markdibarry
Copy link
Contributor

markdibarry commented Jun 28, 2024

@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
Copy link
Contributor Author

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
Copy link
Contributor

markdibarry commented Jun 28, 2024

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
Copy link
Contributor Author

Yeah, my comment over at #87297 (comment) 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
Copy link
Contributor

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 #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
Copy link
Contributor Author

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
Copy link
Contributor Author

alvinhochun commented Jun 30, 2024

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:

  • The square is being rounded to x=1, this part is simple.
  • The camera is not rounded, so it calculates the top-left corner of the viewport to be x= camera.x - (160 / 2) => x= -79.5.
  • If we gloss over the details (assuming we don't have rotation and zoom), the viewport transform is the inverse of the camera transform. This means the canvas items in the viewport is transformed by x= -(-79,5) => x=79.5. Note the negation.
  • The viewport transform gets passed to RendererCanvasCull::render_canvas, then into _cull_canvas_item as p_parent_xform where the rounding takes place:
    parent_xform.columns[2] = (parent_xform.columns[2] + Point2(0.5, 0.5)).floor();
  • Here is where problem occurs: Remember that the viewport transform is inverted? This affects how halfway values are rounded, because floor(-x) == -ceil(x) != -floor(x) (don't ask me for the mathematical proof). The root is now rounded to x=80, which translates the square to x=81, causing the offset in the issue.
    • This also explains why when using round before Replace pixel rounding with floor(x + 0.5) #93740 it only affected the range between 0 < x < 80, because only between this range the signs of the camera top-left and the square positions line up to cause the issue.

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?

@tripodsan
Copy link

tripodsan commented Sep 18, 2024

I don't know the exact root cause, but while I was developing a game for the GBJam, where the viewport is 160x144 and everything should be snapped to pixels, I noticed that the character sometimes jitters (but only when moving in Y direction), when walking along a drag limit. I fixed it by moving the Camera2D node outside the character node and adding:

extends Camera2D
class_name TrackingCamera2D

var target:Player

func _process(delta: float) -> void:
  // set the cameras position to the current player
  global_position = round(target.global_position)

Before:
2024-09-18_18-50-06

After:
2024-09-18_18-53-01

Edit:
actually, after thinking about what the comments above said: it has to do with the drag margins and divisibility of the viewport size. when I choose 0.4 as the margins, it works for X, because 160 * 0.4 = 64, i.e. the drag position is always on an integer. but for Y, 144 * 0.4 = 57.6. probably then using a different rounding than the transform snapping. when I change the drag margin for example to 0.25 it works again.

@akien-mga akien-mga added this to the 4.4 milestone Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants