Comment 10 for bug 1256597

Revision history for this message
Diederik van Lierop (mail-diedenrezi) wrote :

I believe that this has been fixed as of rev. 13333.

Krzysztof, the fix was slightly different though; it was not the "pick state" that was out-of-date, but the bounding box. Also, drawing->update() was already being called just before arena->drawing.pick, in sp_canvas_arena_point(). So I just changed the update() call to also update the bounding box, so

- arena->drawing.update(Geom::IntRect::infinite(), arena->ctx, DrawingItem::STATE_PICK);
+ arena->drawing.update(Geom::IntRect::infinite(), arena->ctx, DrawingItem::STATE_PICK | DrawingItem::STATE_BBOX);

This does not fit well though with the model of updating stuff in the idle handlers, I believe. We're now updating the bounding box explicitly, whereas this could likely have been done earlier if the selector tool (or node tool) would have put this on the idle stack, and if the idle stack was working properly. I'm not sure if the latter is the case though. Let me explain:

1) The selector tools does a requestDisplayUpdate()
2) This allows the bounding box to be invalidated for the relevant items, when _updateDocument() is called in the sp_doc_idle_handler. This also sets need_update to true
3) If needs_update is true, then _updateItem will be called on the DrawingItem, which is all executed in another idle handler: the one of SPCanvasImpl.

Could it be that 3 is called before 2, i.e. SPCanvasImpl::idle_handler is called before sp_doc_idle_handler, and that this is the reason why the bounding box has not yet been updated? This is very tricky to get a grasp of, with all those different signals and functions being called. And if I would get to understand it at some point, then I'm by still not familiar enough with the Inkscape architecture to understand how this all should work instead and what must be changed.

BTW, I've fixed the selector tool to work regardless of this more general drawing->update() fix, but I've not touched your node tool code. Cojnel's analysis is correct, but it should be added that these messages will only be printed if the bounding box changes when moving one of the nodes