Skip to content

compute floating point boundingRect#2554

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:fix-int32-boundingrect
Dec 14, 2022
Merged

compute floating point boundingRect#2554
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:fix-int32-boundingrect

Conversation

@pijyoi
Copy link
Copy Markdown
Contributor

@pijyoi pijyoi commented Dec 14, 2022

Fixes #2550
A few GraphicsItems compute their bounding rectangle with QPicture::boundingRect() -> QRect, probably following the example in customGraphicsItem.py. This causes issues when the data limits exceed the int32 range of a QRect.

This PR fixes the cases where the bounding rectangle can be easily built up from the drawing primitives used.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Dec 14, 2022

Oh no, are we going to have to generate new test images?

@pijyoi pijyoi force-pushed the fix-int32-boundingrect branch from 49a9bfb to 9e126fb Compare December 14, 2022 04:05
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Dec 14, 2022

Oh no, are we going to have to generate new test images?

No, there was some self-referencing in NonUniformImage. The QPicture was used to compute the bounding rectangle; yet while the paint method had not completed drawing onto the QPicture, boundingRect() was called to draw the optional border.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Dec 14, 2022

This LGTM, thanks @pijyoi !

@j9ac9k j9ac9k merged commit 91d215b into pyqtgraph:master Dec 14, 2022
@pijyoi pijyoi deleted the fix-int32-boundingrect branch December 14, 2022 04:48
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Dec 15, 2022

Hmm, the bounding rectangle computed this way ignores the pen width. Practically speaking, it doesn't seem to cause a problem.

It doesn't seem easy to incorporate the pen width into the bounding rectangle, as the pen width is in device pixels.

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.

BarGraphItem is not showing if bar coordinates are huge

2 participants