On Mar 4, 2010, at 7:17 AM, Krzysztof Kosiński wrote:
The idea of re-attaching the floating dialog to a different window is worrying me. Either the dialog should be assigned to some window and stay assigned to it until it's closed, or the dialog should always be assigned to the currently active window. In the first case the desktop should be a construction parameter of the dialog. In the second case using SP_ACTIVE_DESKTOP or an equivalent together with a change signal is OK. If we store the desktop locally and change it using setDesktop() we are at risk of getting out of sync in some obscure code path. I still don't see how an "user experience manager" class might help here.
It doesn't matter whether you use setDesktop() or a construction parameter - there's still a dependency on SPDesktop, and it cannot be removed.
Oh, sorry for the delay on this response. (I just found I had it hiding in my "Drafts" folder)
The purpose of a floating dialog, at least the most common reason, is to work on the "current" view. Even if one has a single document open, it can have multiple views. Through most of it's earlier life Inkscape worked this way and only this way.
A docked panel, on the other hand, should operate only on the view it is embedded in. So we should have two states: "when floating, operate on current" and then "when docked, operate only on the owner". But then again, we have only a single state : "switch to work on the desktop as it is assigned to you". Approaching things with that single state mindset actually makes things easier.
The problem we have is from lower-level components being coded to look "up" the hierarchy outside of themselves with hardcoded assumptions about how they will be used. Since they have been hardcoded to be used in one and only one situation, then they can not reliably work in other situations.
With the introduction of the GDL docking code, this shortcoming in design became apparent. The problem is that the hardcoded assumptions the widgets held inside themselves no longer applied, thus they broke in various ways. The worst of which results in crashing (and I think that was addressed by a work-around of never destructing panels and/or dialogs).
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.
I am glad that reattaching was worrying you. It properly should. That is, your concerns are most likely well justified in this area. Concern is good because it will help us address this potentially buggy area and get things fixed. While it is true that there is risk in changing things to by dynamically set, there is actually also significant risk in using global variables carelessly.
Specifically I've seen problems in the existing code in that large complex functions will use globals multiple times within the whole of the function instead of storing to a local variable at the beginning and working with that variable. In some of the code I've inspected there is a high risk of parts from different desktops being mixed within the run of a single function. We're not seeing things now since we're very single threaded, but as we move more to use multiple cores we will start to trip over these issues.
And I think one big takeaway here is that global variables are very bad for multithreading. We want multithreading. So we need to reduce and/or eliminate the use of global variables.