Skip to content

Disable and comment the prepareForPaint call in ViewBox.update()#2053

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
NilsNemitz:DoNotPrepareForPaint
Nov 6, 2021
Merged

Disable and comment the prepareForPaint call in ViewBox.update()#2053
j9ac9k merged 1 commit intopyqtgraph:masterfrom
NilsNemitz:DoNotPrepareForPaint

Conversation

@NilsNemitz
Copy link
Copy Markdown
Contributor

This addresses #2029, where a recent change to ViewBox was found to result in an unwanted reset of viewing range with each update of data. All the credit for experimentation and triangulating the problem goes to @pijyoi , @j9ac9k and @JeanBilheux .

But since the problem was introduced as part of my PR #1992, I thought I should propose a PR to fix it.

This change reverts the addition of a prepareForPaint() call in ViewBox.update(). This was added since it appeared to result in a more user-friendly failure mode for the basically unsupported case of data with infinite values, at least on OSX. This addition was not part of the main functionality of #1992, but no side effects were known or expected at the time.

But since such side effects clearly exist after all, I believe a revert to the previous state is the best solution.
This was found to solve the issue in #2029 (comment)_

Closes #2029

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 31, 2021

Thanks for this @NilsNemitz

I appreciate the comment you put in the code here; so much of the code in ViewBox.py has so much in the way of subtle impacts.

Only question I have is if there is an "easy" test to add to test for that catches the error that was detailed to test_ViewBox.py. I'll try and do some experimentation today.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Nov 6, 2021

I was trying to do an "easy" test, but noticed the axis coordinates were changing w/ the zoom behavior, but the image was not, so short of some assertImageMatch business; which I'm not really inclined to do, I can't think of a good way to test for this. Given there is an existing/documented issue and we have confidence in this fix, I'll merge this. Thanks @NilsNemitz for the PR!

@j9ac9k j9ac9k merged commit 13df111 into pyqtgraph:master Nov 6, 2021
@NilsNemitz NilsNemitz deleted the DoNotPrepareForPaint branch November 8, 2022 17:07
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.

Bug introduced in setState in version 0.11.x, 0.12.3 (but it's working on 0.12.0!)

2 participants