I could change it to separate it from the rest. But probably i'll need some directions as i'm a newbie.

Some stuff is already separated such as the hotkey handling which is done in kbdnav_keyhttps://github.com/HenriAugusto/pure-data/blob/f5c39cb5cd7ebc101b0b3d9dcde28491b2093b57/src/g_editor.c#L5025 instead of canvas_key (which is already very long).

But there is stuff like the drawing of the rectangles to indicate which in/outlet is selected for example, which is done in glist_drawioforhttps://github.com/HenriAugusto/pure-data/blob/f5c39cb5cd7ebc101b0b3d9dcde28491b2093b57/src/g_text.c#L1365 inside g_texc.c.

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.

So how one should go to keep everything optional? Using preprocessor #if's to include/exclude code during compilation?

________________________________ De: Miller Puckette msp@ucsd.edu Enviado: quarta-feira, 12 de junho de 2019 00:48 Para: Dan Wilcox Cc: Henri Augusto Bisognini; pd-dev Assunto: Re: [PD-dev] First complete keyboard navigation prototype

I agree - I haven't looked at the code but if it can fit easily into Pd's source tree as a run-time option this would be worth adding.

cheers M

On Mon, Jun 10, 2019 at 01:59:35PM +0200, Dan Wilcox wrote:

*Not really feedback, but just a general response.*

One thing I would suggest would be to make this an optional feature in some way. Maybe we can work together with IOhannes and Christof to propose changes to the Pd sources to allow for adding in this kind of functionality without *requiring* it to be built into the core itself. I have been thinking of these kinds of approaches since I see this being a greta feature for the GUI but basically unused for something like libpd. In the latter case, I'd like to simply not compile it and the easiest way would if the main functionality is in a separate set of files that can be left out.

This might be a process that has to happen later on, but something to think of for now in order to balance the addition of new features while maintaining the possibility for a fast, slim core.

...

On Jun 10, 2019, at 10:30 AM, pd-dev-request@lists.iem.at wrote:

Date: Sun, 9 Jun 2019 19:16:36 +0000 From: Henri Augusto Bisognini <msndohenri@hotmail.com mailto:msndohenri@hotmail.com> To: "pd-dev@lists.iem.at mailto:pd-dev@lists.iem.at" <pd-dev@lists.iem.at mailto:pd-dev@lists.iem.at> Subject: Re: [PD-dev] First complete keyboard navigation prototype Message-ID: <CP2PR80MB0386C0D2C66D1CE34F458BE3CC120@CP2PR80MB0386.lamprd80.prod.outlook.com mailto:CP2PR80MB0386C0D2C66D1CE34F458BE3CC120@CP2PR80MB0386.lamprd80.prod.outlook.com>

Content-Type: text/plain; charset="iso-8859-1"

Thanks Christof! Indeed it's already been very nice to use it during my tests.

I'l probably wrap up a small file containing some basic infos about how everything works to make it easier for more people to modify the code and test ideas.

---

Dan Wilcox @danomatika http://twitter.com/danomatika danomatika.com http://danomatika.com/ robotcowboy.com http://robotcowboy.com/

---

Pd-dev mailing list Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev

Please excuse my ignorance on that matter but could you give me a brief explanation of the problem at hand?

________________________________ De: Pd-dev pd-dev-bounces@lists.iem.at em nome de IOhannes m zmölnig zmoelnig@iem.at Enviado: sexta-feira, 14 de junho de 2019 04:37 Para: pd-dev@lists.iem.at 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

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 christof.ressi@gmx.at Enviado: sábado, 15 de junho de 2019 17:11 Para: Henri Augusto Bisognini Cc: pd-dev@lists.iem.at 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" msndohenri@hotmail.com An: "pd-dev@lists.iem.at" pd-dev@lists.iem.at 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 pd-dev-bounces@lists.iem.at em nome de IOhannes m zmölnig zmoelnig@iem.at Enviado: sexta-feira, 14 de junho de 2019 04:37 Para: pd-dev@lists.iem.at 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 Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev

Thanks a lot for your input Christof!

If i may let me ask a couple of question:

1) I've been wandering through the sources and i'm not sure: where should i put the "#def HAVE_KEYBOARDNAV"? Also this value will be set manually in the code or is there anyway to set it via some config file/some other way?

_______________________________________

2) Also i've put a big portion of the code in a g_kbdnav.c file a long with it's forward declarations in g_kbdnav.h.

Why it was in g_editor? Because it uses canvas_connect_with_undo, for example, which is static.

What should i do? Change those methods so they're not static anymore?

Or maybe put a "helper function" inside g_editor.c?

in g_editor.c:

void kbdnav_connect_with_undo(t_canvas *x, t_float index1, t_float outno, t_float index2, t_float inno){ connect_with_undo(x, index1,outno, index2, inno); }

in g_kbdnav.c:

[...] kbdnav_connect_with_undo(...) [...]

Or should i do something else? _______

By the way i wanted to hear more from you guys about the experience when trying the kbdnav prototype! :) Almost no one commented.

Best, Henri.

________________________________ From: Christof Ressi christof.ressi@gmx.at

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" msndohenri@hotmail.com An: "pd-dev@lists.iem.at" pd-dev@lists.iem.at 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 christof.ressi@gmx.at Enviado: sábado, 15 de junho de 2019 17:11 Para: Henri Augusto Bisognini Cc: pd-dev@lists.iem.at 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" msndohenri@hotmail.com An: "pd-dev@lists.iem.at" pd-dev@lists.iem.at 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 pd-dev-bounces@lists.iem.at em nome de IOhannes m zmölnig zmoelnig@iem.at Enviado: sexta-feira, 14 de junho de 2019 04:37 Para: pd-dev@lists.iem.at 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 Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev _______________________________________________ Pd-dev mailing list Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev

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 christof.ressi@gmx.at Enviado: terça-feira, 16 de julho de 2019 14:32 Para: Henri Augusto Bisognini msndohenri@hotmail.com Cc: pd-dev@lists.iem.at pd-dev@lists.iem.at 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" msndohenri@hotmail.com An: "pd-dev@lists.iem.at" pd-dev@lists.iem.at 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 christof.ressi@gmx.at Enviado: sábado, 15 de junho de 2019 17:11 Para: Henri Augusto Bisognini Cc: pd-dev@lists.iem.at 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" msndohenri@hotmail.com An: "pd-dev@lists.iem.at" pd-dev@lists.iem.at 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 pd-dev-bounces@lists.iem.at em nome de IOhannes m zmölnig zmoelnig@iem.at Enviado: sexta-feira, 14 de junho de 2019 04:37 Para: pd-dev@lists.iem.at 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 Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev _______________________________________________ Pd-dev mailing list Pd-dev@lists.iem.at https://lists.puredata.info/listinfo/pd-dev