On Mon, Aug 11, 2008 at 3:34 PM, Peter Clifton <[EMAIL PROTECTED]> wrote:
> On Mon, 2008-08-11 at 13:59 +0200, Bernd Jendrissek wrote:
>> On Mon, Aug 11, 2008 at 8:51 AM, Peter TB Brett <[EMAIL PROTECTED]> wrote:
>> > On Sunday 10 August 2008 01:25:31 Bernd Jendrissek wrote:
>
> [snip]
>
>> typedef struct st_gschem_text_object GSCHEM_TEXT_OBJECT;
>>
>> struct st_gschem_text_object {
>> OBJECT base;
>> PangoLayout *layout;
>> };
>>
>> OBJECT *gschem_new_text(TOPLEVEL *toplevel, ...)
>> {
>> GSCHEM_TEXT_OBJECT *retval;
>> [snip]
>> retval = g_malloc(sizeof (GSCHEM_TEXT));
>> s_basic_init_object(retval, OBJ_TEXT);
>> retval->layout = pango_layout_new(gschem_pango_context);
>>
>> return (&retval->base);
>> }
>>
>> (I haven't even begun on this level of information hiding yet.)
>
> This assumes gschem owns the memory for all objects, which is not
> currently the case. Doing this would not, for example, allow portions of
> gschem to be lib'rified, then used in another app which had its own idea
> of what extra data should be stashed against a text object.
>
> libgeda can free / create objects. I know you're combining this with a
> factory pattern where gschem would do the free'ing, and any associated
> gschem cleanup, but it doesn't scale to where you might have multiple
> different "frontends" looking onto the same objects. (Wanting to inherit
> and extend the _same_ parent object in different ways). I can't think of
> the C++ way this would work either, but then again.. I don't know C++
> particularly well.
You're right, I do have a more complete factory pattern in another of
my branches, but it met with the same resistance as the objections
you're raising here, so for now I just made s_toplevel_new_object() a
shorthand for g_malloc + s_basic_init_object() without smuggling in
the rest of factory.
I can follow your reasoning about the potential desireability of a
many-to-one association between view and model objects, but I fail to
see how this patch you're objecting to makes that any harder. You
can't do what you want to do with a monolithic s_basic_init_object()
either!
> Diverging into mad ideas here, let's imagine say:
>
> gschem -> libgschem, providing a schematic rendering widget.
> gattrib -> libgattrib, providing a spread-sheet attributes widget.
> gschem + gattrib -> UI shell instantiating doing either / both of the
> above.
Well let's get there first. This patch only adds options, not
restrictions. You don't HAVE to use the now-available potential to
specialize OBJECT.
> If the (lib)gschem code has one idea of what each object looks like in
> memory, and the (lib)gattrib code has another, they can't coexist whilst
> dealing with the same objects.
I agree that delegation better supports MVC than inheritance. Please
remember that my example was contrived in haste and if you look
closely enough at it, that particular example of use will have flaws.
> Some options, all ugly in some way or other:
>
> 1.
> libgeda OBJECTs could provide one or more slots to stash arbitrary data
> associated with objects. (void * / gpointer). If we allow multiple
> "frontends", these will have to be associated by string / key / atom.
I think GObject would make this easy, but from previous iterations of
this discussion I know that idea doesn't have much traction here.
> 2.
> gschem could keep its own model associating libgeda objects with its own
> stashed data. TOPLEVEL / GSCHEM_TOPLEVEL is a similar of example, but
> probably suffers a similar issue as your original suggestion because
> gschem owns the TOPLEVEL (or at least assumes it is in charge of
> free'ing it).
My original suggestion suffers no more issues than current libgeda does!
> Option 2 seems more like traditional MVC implementations. Option 1 is
> similar, but slightly more pragmatic. Given we are in control of the
> model, we can ask it nicely to stash pointers to our data, so we don't
> have to keep separate tables of associations between model / view data.
> The view data still exists, and requires similar care to track the
> lifetime of the model object.
Heck, I wouldn't even mind if libgeda's OBJECT definition had extra
void * members specifically for gattrib / gschem / etc. It's not like
libgeda is the system C library with 10000 dependent packages!
>> > What you're doing here actually looks to me to make it much, *much* harder
>> > for
>> > us to eventually change to using an OO toolkit or C++ in the end, by
>> > *adding*
>> > to the huge pile of class-system-unfriendly legacy code we already would
>> > have
>> > to plough through. Hiding definitions/adding accessors has quite the
>> > opposite
>> > effect.
>>
>> I don't see how that conclusion follows from the premises. This patch
>> splits the responsibilities of allocator and constructor, which to me
>> seems to make it *easier* to change to a real OO toolkit / C++. Right
>> now you *can't* have a statically allocated OBJECT. With this patch,
>> you can. [I'm NOT saying that doing so is necessarily a good idea.]
>
> Making the libgeda type opaque with hidden structure definitions and the
> addition of accessors is the right way to make the type opaque.
>
> This doesn't solve the problem of stashing gschem specific view data.
> You've proposed one way of doing this (separating mem allocation, + the
> factory), which would require gschem to own the OBJECT data. This method
> should still work without an opaque libgeda OBJECT type.
I guess I just don't grok why you're objecting to the separation of
allocation + initialization, which is all that this one patch is. How
about another (admittedly unlikely) use case: what if you want to
initialize a statically allocated OBJECT? How would you do that now,
*with current libgeda*?
> Thinking about the pixmap problem a little further, and without reading
> code to check feasibility, I wonder about this:
>
> libgeda keeps filename / raw data buffer, scrap out the pixmap storage.
>
> Gschem does not stash a pixmap directly against each object, instead:
>
> 1. Creates / loads the pixmap from the raw data, or from the specified
> filename.
> 2. Pushes that pixmap into a cache, hashed against the filename, or a
> checksum of the raw data.
> 3. Looks in it's cache for an already loaded pixmap when it comes to
> render an object.
That would take care of PICTURE::original_pixmap. Store a tuple in
the hash to get PICTURE::current_pixmap too.
> Yes.. there are some details in there I'm not 100% clear on, such as
> invalidating the cache if a backing-store file changes, or evicting
> pixmaps when objects are deleted. These would require better lifetime
> tracking / change notification from OBJECTs.
Current libgeda doesn't deal with these issues either (IIRC) so I
wouldn't want to hold a patch to the higher standard of having new
feature X (tracking changes to the image file).
> Even if you had a 1:1 stash between the picture OBJECT and gschem local
> data, you still need to solve the above problems, such as recomputing
> the scaled pixmap to render if the picture size changes, or the filename
> is changed.
This is all doable while maintaining current behaviour. An object
notification system might make it easier to decouple the OBJECT from
the GdkPixbuf, but isn't strictly necessary.
_______________________________________________
geda-dev mailing list
[email protected]
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
