Hi!

I have a question regarding how can i use the pimpl idiom in the kbd navigation 
case.
The editor private members will be defined in g_editor.c.
But since i'm moving the core kbd navigation stuff to it's own file 
(g_kbdnav.c) i must have a way of accessing this t_kbdnav struct on that file.

Currently i've used the solution (example below) of implementing a 
canvas_getkbdnav(t_canvas *x) function on g_editor.c that returns a pointer to 
it's t_kbdnav struct.

Is this okay/good? Why?

Note:
I could make canvas_getkbdnav(t_canvas *x) static and pass a t_kbdnav pointer 
as a parameter from g_editor to the kbdnav stuff. The problem is that i also i 
need to call some kbdnav function from the iemgui files (g_toggle.c, 
g_hslider.c, etc) because they have custom drawing functions that need to call 
some kbdnav logic to draw the in/outlet selection.

____________________________________________________________________________________

----------g_canvas.h----------

typedef struct _editor {
    ...
    void *e_privatedata;
} t_editor;

----------g_editor.c----------

typedef struct _editor_private {
#ifdef HAVE_KEYBOARDNAV
    t_kbdnav kbdnav;
#endif
} t_editor_private;

t_kbdnav* canvas_getkbdnav(t_canvas *x)
{
    ...
}

----------g_kbdnav.h----------

EXTERN t_kbdnav* canvas_getkbdnav(t_canvas *x);

----------g_kbdnav.c----------

void kbdnav_somefunction(t_canvas *x)
{
    t_kbdnav *kbdnav = canvas_getkbdnav(x);
    ...
}


Cheers,
Henri.



________________________________
De: Christof Ressi <[email protected]>
Enviado: terça-feira, 16 de julho de 2019 14:32
Para: Henri Augusto Bisognini <[email protected]>
Cc: [email protected] <[email protected]>
Assunto: Aw: Re: [PD-dev] First complete keyboard navigation prototype

Hi,

> But it appears that actually the size of the struct is deduced while 
> compiling. So if you write an external it is going to think the canvas struct 
> is the same size as it was in the pd header files that were used during 
> compilation.

exactly.

> When using the pointer to implementation idiom, wouldn't the pointer itself 
> change the size of the struct?

yes, it will

> You've said something about that not being a problem when you add it as the 
> last member of the struct but i don't understand why and how that would work.

usually, externals shouldn't care about the *size* the t_editor struct (at 
least I can't think of any use case), so you can get away with adding new 
fields at the end (although it's certainly not recommended). Note that those 
headers aren't really public anyway!

However, appending fields conditionally can lead to problems:

struct Foo {
    int a;
#ifdef FOO_EX
    int c;
#endif
};

Now let's say we need to add another member:

struct Foo {
    int a;
#ifdef FOO_EX
    int c;
#endif
    in b;
};

If the host compiles with FOO_EX defined and the client doesn't, the latter 
will assume a wrong offset for 'b'.

The solution is to add a field for private data *once*. The advantage is that 
a) we can hide the private data and b) we can extend it without worrying about 
compatibility:

struct Foo {
   int a;
   PrivateFoo *p;
};

We can still add public members if needed:

struct Foo {
    int a;
    void *private;
    int b;
};

'private' points to a private data structure that is not be visible to clients. 
There you can conditionally enable members without problems:

struct PrivateFoo {
#ifdef USE_BAR
    struct MyFeature feature;
#endif
};

MyFeature could be in a seperate source file together with your methods and it 
only gets compiled when needed.

Again, have a look at the "t_canvas_private" struct and the "gl_privatedata" 
member of "_glist" (aka "t_canvas") and do the same for "_editor", e.g.:

in g_canvas.h:

typedef struct _editor {
    ...
    void *e_privatedata;
} t_editor;

in g_editor.c:

#ifdef HAVE_KEYBOARDNAV
#include "g_keyboardnav.h"
#endif

typedef struct _editor_private {
#ifdef HAVE_KEYBOARDNAV
    t_keyboardnav keyboardnav;
#endif
} t_editor_private;

the "t_keyboardnav" struct is defined in "g_keyboardnav.h" and its methods 
implemented in "g_keyboardnav.c". Both only get compiled when needed.

Hope this makes sense.

Christof

Gesendet: Dienstag, 16. Juli 2019 um 16:51 Uhr
Von: "Henri Augusto Bisognini" <[email protected]>
An: "[email protected]" <[email protected]>
Betreff: Re: [PD-dev] First complete keyboard navigation prototype
Okay, just help me check if i got it right.

At first i was thinking that when externals, for any reason, used the size of 
the canvas struct (or any other) it would do so in real time. Like calling 
sizeof() and stuff.

But it appears that actually the size of the struct is deduced while compiling. 
So if you write an external it is going to think the canvas struct is the same 
size as it was in the pd header files that were used during compilation.

Is that it?

I don't think i quite get something. When using the pointer to implementation 
idiom, wouldn't the pointer itself change the size of the struct? You've said 
something about that not being a problem when you add it as the last member of 
the struct but i don't understand why and how that would work.

(I don't have a formal background on programming so forgive me if thats obvious 
or something.)


________________________________
De: Christof Ressi <[email protected]>
Enviado: sábado, 15 de junho de 2019 17:11
Para: Henri Augusto Bisognini
Cc: [email protected]
Assunto: Aw: Re: [PD-dev] First complete keyboard navigation prototype

Hi, as IOhannes said, "g_canvas.h" is semi-public in a sense that some 
externals use it (e.g. iemguts). So unless it is absolutely necessary, we 
should avoid breaking binary compatibility.


If the e_kbdnav member is only conditionally enabled with an #ifdef, existing 
externals (or those not compiled with key-nav-support) will see a different 
size of t_editor. This is not much of a problem as long as e_kbdnav is the last 
member of t_editor, but as soon as we add another member, we might run into 
problems, since this last field will be at a different offset.

I think the solution is simple: just add a "void *e_private" member which 
points to some private data where we can put all stuff which we don't want to 
expose the header. (This is called the "PIMPL idiom"). e_kbdnav would be the 
first member of such private data.

IOhannes actually did this with the "gl_privatedata" member in t_canvas to hide 
the undo queue implemention.  The "t_canvas_private" struct currently only has 
a "t_undo" member but it's possible to add/remove/rearrange members at will 
without having to think about binary compatibility issues because it's not in a 
header file.

Christof
Gesendet: Samstag, 15. Juni 2019 um 19:58 Uhr
Von: "Henri Augusto Bisognini" <[email protected]>
An: "[email protected]" <[email protected]>
Betreff: Re: [PD-dev] First complete keyboard navigation prototype
Please excuse my ignorance on that matter but could you give me a brief 
explanation of the problem at hand?


________________________________
De: Pd-dev <[email protected]> em nome de IOhannes m zmölnig 
<[email protected]>
Enviado: sexta-feira, 14 de junho de 2019 04:37
Para: [email protected]
Assunto: Re: [PD-dev] First complete keyboard navigation prototype

On 6/13/19 7:34 PM, Henri Augusto Bisognini wrote:
> Also, in g_canvas.h, inside the "struct _editor" there is a "struct _kbdnav" 
> member. This is the struct that contains the data used in the keyboard 
> navigation.
>

i haven't looked at the actual code, but what you describe here, is that
you are actually changing the size of a quasi-public struct and thus the
memory layout as presented to externals.

which means that a version of Pd that has the keyboard-navigation
enabled is (partly) *binary-incompatible* with a version of Pd that does
not have the keyboard-navigation enabled.

bummer :-(

gfmadr
IOhannes

_______________________________________________ Pd-dev mailing list 
[email protected] https://lists.puredata.info/listinfo/pd-dev
_______________________________________________ Pd-dev mailing list 
[email protected] https://lists.puredata.info/listinfo/pd-dev
_______________________________________________
Pd-dev mailing list
[email protected]
https://lists.puredata.info/listinfo/pd-dev

Reply via email to