-
Notifications
You must be signed in to change notification settings - Fork 194
feat: scene compute_bb
#1024
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
base: main
Are you sure you want to change the base?
feat: scene compute_bb
#1024
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,6 +126,59 @@ impl Scene { | |
| self.encoding.encode_end_clip(); | ||
| } | ||
|
|
||
| /// Compute a rough bounding box of the control points in the scene. | ||
| /// This is not axis-aligned or precise. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not precise means it isn't "tight"; it doesn't wrap the paths, it wraps the control points. |
||
| pub fn compute_bb(&self) -> Rect { | ||
| if self.encoding.path_data.is_empty() { | ||
| return Rect::ZERO; | ||
| } | ||
|
|
||
| let mut min_x = f32::INFINITY; | ||
| let mut min_y = f32::INFINITY; | ||
| let mut max_x = f32::NEG_INFINITY; | ||
| let mut max_y = f32::NEG_INFINITY; | ||
|
|
||
| // Treat consecutive u32 pairs as potential x,y coordinates | ||
| for chunk in self | ||
| .encoding | ||
| .path_data | ||
| .chunks_exact(2) | ||
| .chain(self.encoding.draw_data.chunks_exact(2)) | ||
| { | ||
| let &[x, y] = chunk else { | ||
| continue; | ||
| }; | ||
| let x = f32::from_bits(x); | ||
| let y = f32::from_bits(y); | ||
|
Comment on lines
+148
to
+152
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't valid if there are i16 coordinates. See also #819. |
||
|
|
||
| // Only consider finite values as valid coordinates | ||
| if x.is_finite() && y.is_finite() { | ||
| min_x = min_x.min(x); | ||
| min_y = min_y.min(y); | ||
| max_x = max_x.max(x); | ||
| max_y = max_y.max(y); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, see also #819 - maybe we could return |
||
| } | ||
|
|
||
| // Check glyph positions if any | ||
| for glyph in &self.encoding.resources.glyphs { | ||
| let x = glyph.x; | ||
| let y = glyph.y; | ||
| min_x = min_x.min(x); | ||
| min_y = min_y.min(y); | ||
| max_x = max_x.max(x); | ||
| max_y = max_y.max(y); | ||
| } | ||
|
Comment on lines
+164
to
+171
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't quite follow how this code gets the bounds of the glyphs. If I have a glyph, it won't just be a 0x0 pixel box?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree this code looks wrong. I wasn't sure how to get the bounds of the glyphs cheaply without shaping/etc. so I just took the xy as a control point. Once again this refers to the BB not being precise. |
||
|
|
||
| // If we found no valid coordinates, return zero. | ||
| if min_x.is_infinite() || min_y.is_infinite() || max_x.is_infinite() || max_y.is_infinite() | ||
| { | ||
| return Rect::ZERO; | ||
| } | ||
|
|
||
| Rect::new(min_x as f64, min_y as f64, max_x as f64, max_y as f64) | ||
| } | ||
|
|
||
| /// Draw a rounded rectangle blurred with a gaussian filter. | ||
| pub fn draw_blurred_rounded_rect( | ||
| &mut self, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // Copyright 2024 the Vello Authors | ||
| // SPDX-License-Identifier: Apache-2.0 OR MIT | ||
| //! Simple testing of vello scenes. | ||
| // The following lints are part of the Linebender standard set, | ||
| // but resolving them has been deferred for now. | ||
| // Feel free to send a PR that solves one or more of these. | ||
| #![allow( | ||
| clippy::missing_assert_message, | ||
| clippy::allow_attributes_without_reason | ||
| )] | ||
|
Comment on lines
+4
to
+10
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is entirely new code, and so should follow the usual patterns |
||
| use vello::Scene; | ||
| use vello::kurbo::{Affine, Ellipse, Rect}; | ||
| use vello::peniko::{Brush, color::palette}; | ||
|
|
||
| #[test] | ||
| fn simple_square() { | ||
| let mut scene = Scene::new(); | ||
| scene.fill( | ||
| vello::peniko::Fill::NonZero, | ||
| Affine::IDENTITY, | ||
| &Brush::Solid(palette::css::RED), | ||
| None, | ||
| &Rect::from_center_size((-25., 0.), (50., 50.)), | ||
| ); | ||
| let actual = scene.compute_bb(); | ||
| let expected = Rect::new(-50.0, -25.0, 0.0, 25.0); | ||
| assert_eq!(expected, actual); | ||
| } | ||
|
|
||
| #[test] | ||
| fn simple_ellipse() { | ||
| let mut scene = Scene::new(); | ||
| scene.fill( | ||
| vello::peniko::Fill::NonZero, | ||
| Affine::IDENTITY, | ||
| &Brush::Solid(palette::css::RED), | ||
| None, | ||
| &Ellipse::new((0.0, 0.0), (50.0, 20.0), 0.0), | ||
| ); | ||
| let actual = scene.compute_bb(); | ||
| let expected = Rect::new(-50.0, 50.0, -20.0, 20.0); | ||
| assert_eq!(expected, actual); | ||
| } | ||
|
|
||
| #[test] | ||
| fn rotated_ellipse() { | ||
| let mut scene = Scene::new(); | ||
| scene.fill( | ||
| vello::peniko::Fill::NonZero, | ||
| Affine::IDENTITY, | ||
| &Brush::Solid(palette::css::RED), | ||
| None, | ||
| &Ellipse::new((0.0, 0.0), (50.0, 20.0), 90.0), | ||
| ); | ||
| let actual = scene.compute_bb(); | ||
| let expected = Rect::new(-20.0, -50.0, 20.0, 50.0); | ||
| assert_eq!(expected, actual); | ||
| } | ||
|
|
||
| #[test] | ||
| fn empty_scene() { | ||
| let scene = Scene::new(); | ||
| let actual = scene.compute_bb(); | ||
| let expected = Rect::ZERO; | ||
| assert_eq!(expected, actual); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can it be non-axis aligned? The
Recttype is inherently axis-aligned, as far as I can tell. Should it also return a rotation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My goal isn't to compute an AABB, just a BB since I just need to do basic viewport culling with the BB.
By axis aligned I mean the rect will not rotate to fit the paths it contains better.