Skip to content

Add Tree::max_size to compute actual SVG size #944

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JustForFun88
Copy link

This PR adds Tree::max_size to easily get the final rendered size of an SVG, including all strokes, filters, and transforms.

Motivation

This reduces boilerplate for anyone who needs the actual canvas size, for example when computing a scaling Transform to fit the SVG into a target region and to help avoid memory overflows (see #814).

@LaurenzV
Copy link
Collaborator

LaurenzV commented Aug 1, 2025

Hmm, I'm not sure I'm convinced that we should have this method, it seems very specific. If anything, we should maybe adapt the documentation of Tree::size that this does not refer to the actual bounding box?

@JustForFun88
Copy link
Author

Hmm, I'm not sure I'm convinced that we should have this method, it seems very specific. If anything, we should maybe adapt the documentation of Tree::size that this does not refer to the actual bounding box?

That makes sense. I agree that updating the docs for Tree::size with a note and an example on how to get the actual rendered bounds would definitely help users.

Having a dedicated method like Tree::max_size is not strictly essential, but it does make things more convenient and helps avoid having to think about how to correctly calculate the actual SVG size. But this isn’t very important to me 😆, so I’m happy to go with whatever you think is best for the project

@LaurenzV
Copy link
Collaborator

LaurenzV commented Aug 1, 2025

I think updating the docs would be better, yes.

@RazrFalcon
Copy link
Collaborator

Why do we need a one-line wrapper?
And even then, just keep the current naming convention, aka abs_layer_bounding_box + copy-paste its docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants