Seems the problem is this rather evil bit of code in sp_dtw_desktop_shutdown:
if (doc && (((GObject *) doc)->ref_count == 1)) {
Guess I was right about having exposed an existing bug.
This is wrong for two reasons:
1. objects other than SPDesktops can reference SPDocuments, and there are non-inter-object-reference uses for refcounts: the wrong value being used for the wrong concept
2. GObject::ref_count is sort of an internal field (in the words of the glib API documentation, "All the fields in the GObject structure are private to the GObject implementation and should never be accessed directly.")
The most obvious fix is to specifically keep a count of related active desktops in the SPDocument, and do a similar test with that.
However, should we count SPDesktops, or more generally all SPViews (for example the preview view)?
Is SPDocument necessarily the place to actually keep track of that, or should it be e.g. the Inkscape Application object?
-mental
Quoting MenTaLguY <mental@...3...>:
However, should we count SPDesktops, or more generally all SPViews (for example the preview view)?
Is SPDocument necessarily the place to actually keep track of that, or should it be e.g. the Inkscape Application object?
Nope.
SPDocument is not really the place to keep track of that. It's a model view. It should know that it is dirty, but that's it. All traces of UI awareness should be kept out of it.
Instead, perhaps the ViewManager should track it.
We were hammering out the other day that we'd want something like a ViewSet to hold a set of Views on a given document. The set would keep one or more views together, like for split-pane views. Though there'd be one document per ViewSet, we could have multiple ViewSets per document (split-pane view in one Window, and a zoomed-out 100% view in a separate window).
The ViewManager could track and notice when the last ViewSet capable of 'owning' a document save was closed.
Or... if we don't toss in a ViewManager, then the UIApplication could track it. And at first perhaps we won't have ViewSet, but it would be good to work towards it.
Hmmm.... then there's the case of command-line invocation and such. Maybe we'd want some abstract interface like 'DocumentHolder' that may be a UI ViewSet or may be just an internal non-UI thing.
On Tue, 2005-02-15 at 23:15, jon@...18... wrote:
The ViewManager could track and notice when the last ViewSet capable of 'owning' a document save was closed.
Or... if we don't toss in a ViewManager, then the UIApplication could track it. And at first perhaps we won't have ViewSet, but it would be good to work towards it.
Probably leave it to the Application object for the short-term. That already gets notified of all existing views and documents.
Hmmm.... then there's the case of command-line invocation and such. Maybe we'd want some abstract interface like 'DocumentHolder' that may be a UI ViewSet or may be just an internal non-UI thing.
That would be nice. Currently SPDocuments register themselves directly with Inkscape::Application (the class) on creation, which I hate. Also they do refcounting wrong to avoid confusing the aforementioned buggy close dialog code.
SPDocuments are also currently responsible for Inkscape::Application's lifecycle (via the "keepalive" business in SPDocument), which also strikes me as deeply wrong.
-mental
Quoting MenTaLguY <mental@...3...>:
That would be nice. Currently SPDocuments register themselves directly with Inkscape::Application (the class) on creation, which I hate.
Blech!!!!
Also they do refcounting wrong to avoid confusing the aforementioned buggy close dialog code.
Again, another reason that it is "Blech!!!!"
SPDocuments are also currently responsible for Inkscape::Application's lifecycle (via the "keepalive" business in SPDocument), which also strikes me as deeply wrong.
Yes, it is. We want them to be ignorant model classes in a M-VC pattern, and thus should know nothing of the context of their use, or anything higher. They should only care about their internal state and data.
On Tue, 2005-02-15 at 23:30, jon@...18... wrote:
Ok, so here is the plan:
1. Add a method to Application to answer the question "is the given SPDesktop the sole remaining view of its corresponding document" and rewrite the confirmation dialog code to use that
(this will fix bulia's favorite bug right away)
2. Stop SPDocument registering itself with Application; instead, Application will simply observe the desktops to see which documents they reference and build/update its document list from that instead.
3. Remove Application lifecycle management from SPDocument and put it someplace else (probably have SPViews ref the Application object, and have commandline usage be a special case that simply unrefs the Application when it is done).
-mental
participants (2)
-
unknown＠example.com -
MenTaLguY