On Apr 3, 2010, at 11:29 AM, Jon Cruz wrote:
Now, it is true that these meta components are dependent on an SPDesktop. However, they do not need to depend on "fetch the desktop pointed to by the global variable held in a known location". Instead it is easy to apply dependency injection (though we probably want to stick with manual instead of framework driven). The *KEY* yet subtle difference here is in changing things away from "a dialog reaching out and grabbing the global pointer desktop" to instead have "a dialog uses the desktop that is handed to it"
http://en.wikipedia.org/wiki/Dependency_Injection
Now, I should probably point out that we are *very* close to having those. That is, with just a bit of refactoring we can reach successful dependency injection on the dialogs and panels.
There is an update on this point. In cleaning up the Fill & Stroke dialog so I could get rid of the copy-n-paste extra class (along with the plethora of rogue tile bug pattern occurrences) I was able to convert it to be properly dynamic and work nicely no matter if it were floating or docked, single or multiple. (This happened to also fix bug #270623 ""Fill and Stroke" panel is mirrored between open windows")
I now have that tracking extracted into a stand-alone class, and have already reused it in a different panel. That new code is at
src/ui/dialog/desktop-tracker.h src/ui/dialog/desktop-tracker.cpp
Aside from fixing the immediately visible problems such as bug #270623, other improvements also happened as a consequence of the change. One of these is that prior to this a change in one view of one document would cause a cascade of event processing in all other dialogs for all views in all documents. Things had been all wiring themselves to the global inkscape application instance and listening for changes there. After switching to the new DesktopTracker the updated dialogs only are invoked when their specific desktop has a change (in selection, pertinent contents, document reload, etc).
And of course there is the added benefit of getting things a little more multithreading friendly. That will be good too.
So far I've not gone out to switch all of our dialogs over to this paradigm. Krzysztof expressed proper concern that we might expose some latent bugs in the process of switching. Since we are very close to 0.48, we should probably only change code that is actively being worked on, so we touch only things that we are looking at and are observing the behavior of.
Oh, and if anyone does happen to be working in any code that happens to use SP_ACTIVE_DESKTOP or SP_ACTIVE_DOCUMENT, one minor tweak with a lot of benefit would be to make sure those are only used at the top of a function and are stored to a local variable. Then the function can work against that local variable and gain a bit of safety. And that will make a latter pass to switch things cleaner that much easier.
Thanks.