Don`t change current visibility flag of rubberband on updates #1972
Conversation
|
I dont find any relation of QgsRubberBand and failed TestQgsComposerPicture::pictureRemoteUrl(). |
|
I've restarted travis build, in case it was a temporary glitch. |
| @@ -567,7 +567,7 @@ void QgsRubberBand::updateRect() | |||
| QgsRectangle rect( topLeft.x(), topLeft.y(), topLeft.x() + r.width()*res, topLeft.y() - r.height()*res ); | |||
|
|
|||
| setRect( rect ); | |||
| setVisible( true ); | |||
| setVisible( isVisible() ); | |||
There was a problem hiding this comment.
Does this ever change the state ? isn't it equivalent to just drop the line ?
|
What is expected behavior after creation of rubberband? According to Qt docs QGraphicsItem is visible by default (Items are visible by default; it is unnecessary to call setVisible() on a new item.), but QgsRubberBand ctor calls reset(), which set visibility to false. Should we always manually set visibility to true? Or should it be set on addGeometry() call? |
|
Good question, but better asked on the mailing list.
I'm not really a qgis extension coder to know what to expect...
|
Visibility behavior stays the same as in QGIS 2.6 Test included
Don`t change current visibility flag of rubberband on updates
|
I like this new patch better, thanks. |
|
Actually, I found a missing bit here. The :updateRect function still calls setVisible(false) if there are no points, is that expected ? I guess it should just never change visibility instead ? |
|
Should you add a test that adds an empty geometry ? |
|
Sorry, I see the test is there, all fine here then. |
If we setting visibility to true on valid geometry then we should set it to false on invalid/empty geometry. And this is not break any user stuff, coz QGIS 2.6 (and early, i think) works that way. |
|
Backported in 2.8 branch as 7210c4a |
If you hide rubberband and then zoom map, rubberband will be shown again.