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

percentage based default for boundingRect() is problematic #450

Open
sjparker opened this issue Oct 28, 2024 · 2 comments
Open

percentage based default for boundingRect() is problematic #450

sjparker opened this issue Oct 28, 2024 · 2 comments

Comments

@sjparker
Copy link

The node’s size plus some additional margin around it to account for drawing effects (for example shadows) or node’s parts outside the size rectangle (for example port points).

The default implementation returns QSize + 20 percent of width and heights at each side of the rectangle.

For resizable nodes, this percentage ends up being pretty significant in terms of hit box detection for selecting nodes. In the stated example of the types of effects or node parts, namely shadows, a fixed padding would probably be more appropriate as drop shadows shouldn't be changing the amount of offset when a node scales. There also doesn't seem to be an easy way to override this especially when using the DataFlow* classes. I would be happy to provide a PR but wanted to discuss first.

@paceholder
Copy link
Owner

You are right, the default values might be sub-optimal because I did not delelop any large realistic application based on this framework.

I think the best work-around would be making the code extensible.
Perhaps something similar to cusom "node painter" or "connection painter" in BasicGraphicsScene could be a solution.

I.e. we could have a pair of functions nodeGeometry() and setNodeGeometry(unique_ptr<AbstractNodeGeometry> && g)

What do you think?

Best,
Dimitry

@sjparker
Copy link
Author

setNodeGeometry was exactly what I was searching for when I posted this. I think that's the way to go. I'll do it but it might take a bit for me to send the PR. Thanks again for a great little library.

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

No branches or pull requests

2 participants