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

Line widths grow as 2D view shrinks #6215

Closed
emilk opened this issue May 3, 2024 · 5 comments · Fixed by #6239
Closed

Line widths grow as 2D view shrinks #6215

emilk opened this issue May 3, 2024 · 5 comments · Fixed by #6239
Assignees
@Wumpf
Labels
🪳 bug Something isn't working 🔺 re_renderer affects re_renderer itself 🦟 regression A thing that used to work in an earlier release ui concerns graphical user interface
Milestone
0.16

Comments

@emilk
Copy link
Member

emilk commented May 3, 2024 •
edited
Loading

image

If I specify the line widths in scene units it works, but specifying them in ui points causes them to be way too large.
@emilk emilk added 🪳 bug Something isn't working ui concerns graphical user interface 🦟 regression A thing that used to work in an earlier release labels May 3, 2024
@emilk emilk added this to the 0.16 milestone May 3, 2024
@emilk emilk assigned emilk and unassigned emilk May 6, 2024
@emilk
Copy link
Member Author

emilk commented May 6, 2024

TODO: check if this is a regression or old bug

@emilk emilk added the 🔺 re_renderer affects re_renderer itself label May 6, 2024
@emilk
Copy link
Member Author

emilk commented May 6, 2024

I suspect RectTransform, without proof. Do we have a test for this @Wumpf ?

@Wumpf
Copy link
Member

Wumpf commented May 6, 2024

RectTransform has a unit test, but since you're saying this is about ui widths this must be about the interaction with pixels_per_point for which we don't have any testing

@Wumpf
Copy link
Member

Wumpf commented May 6, 2024

issue doesn't repro for me. Lines stay constantly at 8pixel for 2ui points at 200% screen scaling which is what I would have expected
image

@Wumpf
Copy link
Member

Wumpf commented May 6, 2024

missunderstanding on my side: changing the size of the viewport causes the issue as described!

@Wumpf Wumpf self-assigned this May 6, 2024
@Wumpf Wumpf mentioned this issue May 6, 2024
Merged
5 tasks
Wumpf added a commit that referenced this issue May 8, 2024
@Wumpf
Fix incorrect ui size rendering for vertically narrow 2d views (#6239)
9189b96 
### What

* Fixes #6215

The problem was that we used the max of x & y scale of the viewport
transform, not taking account that **y**-pixel size was derived from the
projection matrix which operates on a fixed aspect ratio in these cases.
The moment the viewport transform had a higher x scale than y (i.e.
making more things visible in the horizontal), the scale factor would
flip around and be incorrect for the unchanging y pixel size.

Note that the line `tan_half_fov * 2.0 / resolution` produces a
non-square size, since we keep the `aspect_ratio` of the projection
matrix fixed (an arbitrary value if no camera is present, or the camera
is there is one around). This becomes square again (under current
restrictions for not stretching non-uniformly) via
`pixel_world_size_from_camera_distance *
config.viewport_transformation.scale()`.


https://github.com/rerun-io/rerun/assets/1220815/5fe3742d-2bd6-4873-9c1f-8ab552086662


---

This was technically a pre-existing issue but I don't know how to repro
it on 0.15, so excluding from changelog.

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6239?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6239?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6239)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Wumpf Wumpf

Labels
🪳 bug Something isn't working 🔺 re_renderer affects re_renderer itself 🦟 regression A thing that used to work in an earlier release ui concerns graphical user interface
Projects
None yet
Milestone
0.16
Development

Successfully merging a pull request may close this issue.

Fix incorrect ui size rendering for vertically narrow 2d views rerun-io/rerun
2 participants
@emilk @Wumpf