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.

Sign up for GitHub

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

Jump to bottom

Regression with custom projections #16856

Closed
rparrett opened this issue Dec 17, 2024 · 10 comments · Fixed by #17592
Closed

Regression with custom projections #16856

rparrett opened this issue Dec 17, 2024 · 10 comments · Fixed by #17592
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples O-Linux Specific to the Linux desktop operating system P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Investigation This issue requires detective work to figure out what's going wrong

Comments

@rparrett
Copy link
Contributor

rparrett commented Dec 17, 2024 •
edited
Loading

Bevy version

main, bisected to #16828

Relevant system information

SystemInfo { os: "MacOS 14.5 ", kernel: "23.5.0", cpu: "Apple M1 Max", core_count: "10", memory: "64.0 GiB" }
AdapterInfo { name: "Apple M1 Max", vendor: 0, device: 0, device_type: IntegratedGpu, driver: "", driver_info: "", backend: Metal }

Scale Factor 2

What you did

cargo run --example projection_zoom.

See also orthographic, camera_sub_view, custom_primitives.

What went wrong

Projection is noticeably off-center.

Image

Additional information

Caught by bevy-example-runner.

https://pixel-eagle.com/project/b25a040a-a980-4602-b90c-d480ab84076d/run/5991/compare/5988?screenshot=3D%20Rendering/orthographic.png

I suspect that rounding needs to happen after scaling.
@rparrett rparrett added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Dec 17, 2024
@rparrett rparrett added this to the 0.15.1 milestone Dec 17, 2024
@BenjaminBrienen BenjaminBrienen added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! A-Rendering Drawing game state to the screen D-Straightforward Simple bug fixes and API improvements, docs, test and examples P-Regression Functionality that used to work but no longer does. Add a test for this! and removed S-Needs-Triage This issue needs to be labelled labels Dec 25, 2024
@mockersf
Copy link
Member

I can't reproduce #16773 when reverting #16828, could someone else check if it's still happening?

There's no way to add rounding like #16828, even after scaling, that won't produce a position change

@mockersf mockersf modified the milestones: 0.15.1, 0.15.2 Dec 26, 2024
@rparrett
Copy link
Contributor Author

I can't reproduce either

on Mac (m1) or Windows 11 (4080). I also tried WGPU_BACKEND=dx12 on windows. I can't rule out me not understanding the repro.

I think we need linux people to test.

@BenjaminBrienen
Copy link
Contributor

FYI @musjj

@BenjaminBrienen BenjaminBrienen added S-Needs-Investigation This issue requires detective work to figure out what's going wrong and removed S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! labels Dec 26, 2024
@musjj
Copy link
Contributor

musjj commented Dec 26, 2024

Does this not reproduce for you?: https://github.com/musjj/nearest_filter_bug (try to resize the window resolution to odd numbers).

This is my adapter info:

AdapterInfo {
    name: "AMD Radeon RX 550 / 550 Series (RADV POLARIS12)",
    vendor: 4098,
    device: 27039,
    device_type: DiscreteGpu,
    driver: "radv",
    driver_info: "Mesa 24.2.6",
    backend: Vulkan,
}

I'm on Linux with Gnome. I can reproduce the issue both with the X11 and Wayland backend.

@mockersf
Copy link
Member

Does this not reproduce for you?: https://github.com/musjj/nearest_filter_bug (try to resize the window resolution to odd numbers).

Nop, not on macOS

@BenjaminBrienen BenjaminBrienen added the O-Linux Specific to the Linux desktop operating system label Dec 26, 2024
@hukasu
Copy link
Contributor

hukasu commented Jan 28, 2025

SystemInfo { os: "Linux (NixOS 24.11)", kernel: "6.6.69", cpu: "Intel(R) Core(TM) i7-14700KF", core_count: "20", memory: "31.2 GiB" }

7aeb1c5
Image

1030a99
Image

@rparrett
Copy link
Contributor Author

rparrett commented Jan 28, 2025 •
edited
Loading

SystemInfo { os: "Linux (NixOS 24.11)", kernel: "6.6.69", cpu: "Intel(R) Core(TM) i7-14700KF", core_count: "20", memory: "31.2 GiB" }

Thanks for testing. What we really need to know is whether #16773 is still an issue after reverting #16828. And if so, if we can fix that issue in a way that doesn't cause the regression documented in this issue. It seems like that old issue could potentially be platform specific, or a driver issue or something.

@hukasu
Copy link
Contributor

hukasu commented Jan 28, 2025

I'm not seeing the type of tearing on the image as #16773 is showing, but the text is a bit fuzzy, the right edge of the sprite does have some fuzziness though

1030a99
Image

b039bf6
(rounding origin_x and origin_y
Image

local change
(rouding projection_width and projection_height)

Image

Image

@hukasu
Copy link
Contributor

hukasu commented Jan 28, 2025

None of the branches i tested i could replicate the tearing shown on #16773, and main + reverting ae52222 appears the same as the ones without rouding

Image

Image

Image

@hukasu
Copy link
Contributor

hukasu commented Jan 28, 2025

one suggestion is changing from Rect::new to Rect::from_center_half_size on OrthographicProjection::update, so that if there is an error in the calculation, at least the center will always be correct, or is there a case where not being centered is correct?

This was referenced Jan 28, 2025
Open
Merged
github-merge-queue bot pushed a commit that referenced this issue Feb 2, 2025
@hukasu
Revert "Fix rounding bug in camera projection (#16828)" (#17592)
55283bb 
This reverts commit ae52222.

# Objective

Fixes #16856

## Solution

Remove rounding from `OrthographicProjection::update`, which was causing
the center of the orthographic projection to be off center.

## Testing

Ran the examples mentioned on #16856 and code from #16773

## Showcase
`orthographic` example

![image](https://github.com/user-attachments/assets/d3bb1480-5908-4427-b1f2-af8a5c411745)

`projection_zoom` example

![image](https://github.com/user-attachments/assets/e560c81b-db8f-44f0-91f4-d6bae3ae7f32)

`camera_sub_view` example

![image](https://github.com/user-attachments/assets/615e9eb8-f4e5-406a-b98a-501f7d652145)

`custom_primitives` example

![image](https://github.com/user-attachments/assets/8fd7702e-07e7-47e3-9510-e247d268a3e7)

#16773 code

![image](https://github.com/user-attachments/assets/1b759e90-6c53-4279-987e-284518db034b)
@mockersf mockersf removed this from the 0.15.2 milestone Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples O-Linux Specific to the Linux desktop operating system P-Regression Functionality that used to work but no longer does. Add a test for this! S-Needs-Investigation This issue requires detective work to figure out what's going wrong
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Revert "Fix rounding bug in camera projection (#16828)" hukasu/bevy
5 participants
@rparrett @hukasu @mockersf @BenjaminBrienen @musjj