Make some Fl_Tree_Item methods virtual?

[Tim Cuatt]

Hi all,

I'm wondering if one can change the draw_vertical_connector and draw_horizontal_connector methods to virtual in Fl_Tree_Item?

If this is straightforward could we please request this as a feature in 1.4.X? We just want to mess more with the per-item methods in an Fl_Tree. If this sounds too tricky we'll work out another solution so we don't have to maintain a parallel branch for all time; I don't know if that will mess with the ABI too much for instance.

We'd be happy to develop and test this implementation over here and send over the changes if it sounds like a good fit with the toolkit.

Best,

-Tim C

[Greg Ercolano]

On 3/26/25 15:11, Tim Cuatt wrote:

Hi all,

I'm wondering if one can change the draw_vertical_connector and draw_horizontal_connector methods to virtual in Fl_Tree_Item?

    Yes, it can be done.

    BTW, discussions about changing the internals is best done on the fltk.coredev group, as things can quickly go towards discussions of the internals, which can get too technical for the general FLTK use audience, so please consider that for future suggestions.

    Changing methods from protected to virtual will break ABI, from what my research shows, because it affects the virtual table, which itself is part of the ABI.

    That said, we have ways to enable ABI breaking features in mid-stream maintenance releases via the FLTK_ABI_VERSION macro, so it can certainly be added that way.

    I'm not against it, but I await feedback/votes from other devs to hear their concerns (beyond ABI macro protections), if any.

    It makes sense I suppose, because the "connector lines" are the one part of the tree items you can't really redefine easily; these are the connection lines drawn between the items when the tree is opened.

We'd be happy to develop and test this implementation over here and send over the changes if it sounds like a good fit with the toolkit.

    I'd suggest going on github and providing a PR. You don't have to implement the ABI protection macros, we can do that, but just show what changes you're looking for, and perhaps an example or two in the examples/ directory that show exercising the feature in the way you expect.

    Currently the Fl_Tree_Item methods draw_vertical_connector() and draw_horizontal_connector()
    are protected non-virtual.

    It wouldn't be hard to make an ABI protection macro that allows these to be protected virtuals.
    To access this during the FLTK 1.4.x maintenance stream, you'd simply customize the ABI version macro setting and rebuild FLTK to enable such features. There's a README.abi-version.txt file on how to do this.

    The only virtual method currently in Fl_Tree_Item (besides the destructor) is the draw_item_content() method, which is important for creating Fl_Tree_Item derived custom classes that define drawing the item. This method is similar in purpose to overriding the Fl_Widget::draw() method in derived classes.

    There are currently three options for the connector lines:

• None
• Solid lines
• Dotted lines

    When I designed Fl_Tree, I thought that should cover all the bases, but apparently you have something specific in mind, so please include that in your example; perhaps that too can be incorporated as a new default if it's considered something a lot of people would want.

[Ian MacArthur]

On Thursday, 27 March 2025 at 05:01:39 UTC Erco wrote:

    Currently the Fl_Tree_Item methods draw_vertical_connector() and draw_horizontal_connector()

    are protected non-virtual.

    It wouldn't be hard to make an ABI protection macro that allows these to be protected virtuals.
    To access this during the FLTK 1.4.x maintenance stream, you'd simply customize the ABI version macro setting and rebuild FLTK to enable such features. There's a README.abi-version.txt file on how to do this.

In the short term, if they are protected rather than private, can you derive your own tree-item class from Fl_Tree_Item and then change them there? 

 

[Albrecht Schlosser]

I'm mainly replying to technical aspects of development of the FLTK library, not the details of Fl_Tree, but see the last paragraph for some Fl_Tree related notes.

On 3/27/25 06:01 Greg Ercolano wrote:

On 3/26/25 15:11, Tim Cuatt wrote:

I'm wondering if one can change the draw_vertical_connector and draw_horizontal_connector methods to virtual in Fl_Tree_Item?

    Yes, it can be done.

    Changing methods from protected to virtual will break ABI, from what my research shows, because it affects the virtual table, which itself is part of the ABI.

Yep, that's true.

    That said, we have ways to enable ABI breaking features in mid-stream maintenance releases via the FLTK_ABI_VERSION macro, so it can certainly be added that way.

    I'm not against it, but I await feedback/votes from other devs to hear their concerns (beyond ABI macro protections), if any.

My concerns are that this should not be done in FLTK 1.4.x (using ABI guards) because 1.4 should get only bug fixes (it's in maintenance mode). Changing this in FLTK 1.5 (the current `master` branch) would be the preferred.

We'd be happy to develop and test this implementation over here and send over the changes if it sounds like a good fit with the toolkit.

    I'd suggest going on github and providing a PR.

If you do so, please provide the PR related to the master branch. Please create your own branch and commit in that branch (not on master) for easier maintenance.

You don't have to implement the ABI protection macros, we can do that, but just show what changes you're looking for, and perhaps an example or two in the examples/ directory that show exercising the feature in the way you expect.

Since this should IMHO be done in the 1.5 branch (master) we don't need ABI macros anyway. If we ever came to the conclusion that the new feature should be backported to 1.4, then we will need to add the ABI guards only in branch-1.4. But this should only be done after the code has stabilized in `master` (if at all, see above).

@OP (Tim Cuatt): That said, we are always interested in patches and contributions. Please take care to use our coding style (spaces, no tabs, indent by 2, etc.). You should also consider what Ian suggested: is it possible to make the methods in question protected (non-virtual) so you can derive your own class? If the latter would be chosen, then I believe that it would be feasible to change the methods to `protected` under an ABI guard in 1.4 [1] (as an exception to the general rule). If you think that we should change more methods from private to protected or make them virtual, then please suggest this as well. This can be done in the discussion part of the PR before it's implemented (if such changes would be accepted by the FLTK team).

@Greg: You are the one who knows best what all these methods in Fl_Tree do, and maybe you can decide which ones should be protected and/or virtual. My point here is: if we touch the interface (API) then we should "do it right" (TM) and consider maybe more methods that could be interesting for users of the class to modify. Whether they need to be 'virtual' or not should be decided individually. Methods like drawing the connector lines might need to be virtual, even if users derive their own classes because they can be called internally from other drawing methods - but that's only an educated guess...


[1] I'm not sure if making `private` methods `protected` would break the ABI because it adds another method (makes it "visible"), and this does AFAICT not break the ABI - unless it's also made virtual.

[Greg Ercolano]

    I think because those two methods are (currently) non-virtual, you can't override them in a way that the tree can make use of them. Kinda the purpose of making them virtual, so the tree can access the user's overridden methods.
   

[Bill Spitzak]

I think if this is wanted it should be done by overriding draw(). Those are just internal details and the desired graphics may not map that well to these calls.

--
You received this message because you are subscribed to the Google Groups "fltk.general" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkgeneral...@googlegroups.com.
To view this discussion visit https://groups.google.com/d/msgid/fltkgeneral/4d860c6f-97c8-45ca-ad55-260274f7f0a4%40seriss.com.

[Greg Ercolano]

On 3/27/25 10:49 AM, Bill Spitzak wrote:

I think if this is wanted it should be done by overriding draw(). Those are just internal details and the desired graphics may not map that well to these calls.

    Ya, that's why I didn't make them virtual, I considered them "internal" methods.

    However, in the case of Fl_Tree, the Fl_Tree_Item's are not themselves Fl_Widgets (similar in design to Fl_Browser, where the items are a separate private class/struct). The draw_item_content() method is the pseudo equivalent of draw(), but only draws the item's content, not the connectors and does not handle the details of collapsing and such.

    I made the items a public class so their content could be customized, and also one can add Fl_Widgets to the item as a pointer, so that actual FLTK widgets like buttons and such can be attached to the items.

[Greg Ercolano]

    The attached example program (tree-custom-connectors.cxx) demonstrates the issue by creating a custom Fl_Tree_Item class called MyTreeItem which draws a grid as the content, and attempts to also reimplement the draw_vertical_connector()/draw_horizontal_connector() methods, wanting to draw red connection lines in place of the default dotted lines.

    On the left: the app built against FLTK, where the methods are non-virtual, causing the overrides to be ignored.
    On the right is a hacked FLTK, where the two methods made virtual, showing the "red connection lines", indicating the override is working:

[Rob McDonald]

Thanks Greg, you've nailed the use case and our concerns.

I'm working with Tim on this little project.

One ABI preserving approach would be to add a bunch more options to how the lines are drawn -- add colors, bold lines, double lines, etc...  Then, the only visible change would be some new valid enum values.

However, the other part of the use case that misses is the ability to change the line style on a per-item basis.  You've achieved this above by using a different derived type for the MyTreeItem's than the Flinstones and Superjail items.

Right now, the line style is set by a Tree-wide configuration variable.

An ABI preserving way to get per-item styles would be to extend the built-in item class to have a local style configuration variable.  It could (for example) default to -1 -- which would tell it to use the global style.  Otherwise, it would use the local style.

So, from our point of view, there are two solutions....

1) Make the methods virtual -- breaks the ABI, but it allows us to do whatever we want on our side pretty simply.

2) Possibly ABI preserving approach:

2a) Add more line styles -- at least bold and double lines, colors would be a nice add.

2b) Add support for per-item style choices.

One other possible required use case would be to use different styles for the horizontal / vertical segments of the line.  I.e. perhaps you want the vertical line to be solid everywhere, but the horizontal line to be dotted / solid on a per-item basis.  This could be readily achieved by making the methods virtual.  Otherwise, there would need to be two per-item line style configuration flags.

Best,

Rob

 

[Albrecht Schlosser]

On 3/27/25 19:43 Greg Ercolano wrote:

    The attached example program (tree-custom-connectors.cxx) demonstrates the issue by creating a custom Fl_Tree_Item class called MyTreeItem which draws a grid as the content, and attempts to also reimplement the draw_vertical_connector()/draw_horizontal_connector() methods, wanting to draw red connection lines in place of the default dotted lines.

    On the left: the app built against FLTK, where the methods are non-virtual, causing the overrides to be ignored.
    On the right is a hacked FLTK, where the two methods made virtual, showing the "red connection lines", indicating the override is working:

Greg, I also changed my FLTK (1.5) version as described, i.e. by *only* making the two methods virtual, and I can confirm the effect you posted above in the image (as was to be expected).

If this is the only change needed to achieve what Rob and Tim request, then I believe it's sensible to make these two methods virtual in 1.5 and to do the same under an ABI guard (#if FL_ABI_VERSION >= 10403) in branch-1.4.

@Rob and @Tim: the only caveat would be that I don't plan to make a new public release (which would be 1.4.3) for this tiny change, i.e. it would be available only in Git for some time. There are presumably more patches to come for branch-1.4, and then this change would be included in the next release 1.4.3. Would you agree that this is a valid solution for you?

Note: Our build procedure does not allow to set the ABI version (e.g. 10403) higher than the current API version (e.g. 10402) [1]. The consequence would be that we need to prepare the Git branch (i.e. update version numbers) for release 1.4.3 even if it's not (yet) released [1]. We can do that, but it would need some extra work. I'd volunteer to do that since we will very likely see release 1.4.3 in the near future anyway, but there are some concerns regarding the maintenance of Debian, but that's another story (inclusion of 1.4.2 is in progress).

[1] Another option would be that Rob/Tim patch their local FL/Enumerations.H (which implements the limits). Just setting it (by CMake) in FL/fl_config.H is NOT sufficient.

[Rob McDonald]

If this is the only change needed to achieve what Rob and Tim request, then I believe it's sensible to make these two methods virtual in 1.5 and to do the same under an ABI guard (#if FL_ABI_VERSION >= 10403) in branch-1.4.

@Rob and @Tim: the only caveat would be that I don't plan to make a new public release (which would be 1.4.3) for this tiny change, i.e. it would be available only in Git for some time. There are presumably more patches to come for branch-1.4, and then this change would be included in the next release 1.4.3. Would you agree that this is a valid solution for you?

Yes, this would work for us.  While we'd love to be on an official upstream release, we've shipped dev versions for years through the 1.3 and 1.4 process.

Rob

 

[Albrecht Schlosser]

On 3/28/25 16:41 Rob McDonald wrote:

If this is the only change needed to achieve what Rob and Tim request, then I believe it's sensible to make these two methods virtual in 1.5 and to do the same under an ABI guard (#if FL_ABI_VERSION >= 10403) in branch-1.4.

@Rob and @Tim: the only caveat would be that I don't plan to make a new public release (which would be 1.4.3) for this tiny change, i.e. it would be available only in Git for some time. There are presumably more patches to come for branch-1.4, and then this change would be included in the next release 1.4.3. Would you agree that this is a valid solution for you?

Yes, this would work for us.

Great, thanks for confirmation.

While we'd love to be on an official upstream release, we've shipped dev versions for years through the 1.3 and 1.4 process.

Note that there are new activities concerning release 1.4.3 because this may be needed for the next Debian release which has really high priority for us. Please follow GitHub Issue #1238 if you are interested. Solution of this issue would definitely trigger a new release, i.e. 1.4.3, as soon as possible.
https://github.com/fltk/fltk/issues/1238

[Rob McDonald]

On Friday, March 28, 2025 at 10:16:36 AM UTC-7 Albrecht-S wrote:

Note that there are new activities concerning release 1.4.3 because this may be needed for the next Debian release which has really high priority for us. Please follow GitHub Issue #1238 if you are interested. Solution of this issue would definitely trigger a new release, i.e. 1.4.3, as soon as possible.

https://github.com/fltk/fltk/issues/1238

Do you want me to put together a PR for this change -- or as it is so trivial and you've already made the change once in your test branch -- there is no need.

Thanks again for the help,

Rob

 

[Greg Ercolano]

On 3/28/25 6:25 AM, 'Albrecht Schlosser' via fltk.general wrote:

Greg, I also changed my FLTK (1.5) version as described, i.e. by *only* making the two methods virtual, and I can confirm the effect you posted above in the image (as was to be expected).

    OK, I'll apply this change to 1.5.x and reference this thread in the commit
    so there's something to show history, as there's no github issue for this.

    I'll include some small doc changes. I have something in the wings already,
    and I'm just a 'git push' away from committing.

@Greg: You are the one who knows best what all these methods in Fl_Tree do, and maybe you can decide which ones should be protected and/or virtual. My point here is: if we touch the interface (API) then we should "do it right" (TM) and consider maybe more methods that could be interesting for users of the class to modify.

    Right, though it's hard to anticipate.. or I find it so.

    I think the general rule you and I have agreed on: you can always open things later,
    but can't close things once they're open.

    So we've been careful not to "open" any methods we think might change in the future,
    or are "too internal".

Whether they need to be 'virtual' or not should be decided individually. Methods like drawing the connector lines might need to be virtual, even if users derive their own classes because they can be called internally from other drawing methods - but that's only an educated guess...

    I would have thought these two methods would be "too internal" to override
    because of the amount of internal details needed in the method arguments,
    but apparently it's desireable, so here we are.

    I think if I'd anticipated user customization beyond what is already "open",

[Greg Ercolano]

    Regarding adding virtual, I'll handle this; expect a commit to 1.5.x today.

    As I believe Albrecht mentioned in a previous reply, he didn't think this should be
    added to 1.4.x as an ABI feature, so I'll leave that out.

[Albrecht Schlosser]

On 3/28/25 20:53 Greg Ercolano wrote:

On 3/28/25 10:41 AM, Rob McDonald wrote:

On Friday, March 28, 2025 at 10:16:36 AM UTC-7 Albrecht-S wrote:

Note that there are new activities concerning release 1.4.3 because this may be needed for the next Debian release which has really high priority for us. Please follow GitHub Issue #1238 if you are interested. Solution of this issue would definitely trigger a new release, i.e. 1.4.3, as soon as possible.

https://github.com/fltk/fltk/issues/1238

Do you want me to put together a PR for this change -- or as it is so trivial and you've already made the change once in your test branch -- there is no need.

    Regarding adding virtual, I'll handle this; expect a commit to 1.5.x today.

Thanks. FTR: Greg's commit is 9e1e043af35e979f5ad0824fb74f6e7fab57be7d

    As I believe Albrecht mentioned in a previous reply, he didn't think this should be
    added to 1.4.x as an ABI feature, so I'll leave that out.

This was my initial thought, but later, when I realized that it was such a trivial modification, I changed my mind.

Anyway, I can do the backport, particularly because an ABI guard against ABI version 10403 (1.4.3) would not work w/o changing the API version to 1.4.3 as well (see my previous message).

I'm thinking about changing the max-allowed ABI version to (API version + 1). This would allow to set the ABI version of the *next* release so ABI guards for that version would allow that code to be compiled. Then we could leave the API version at 1.4.2 (as long as it's not released), but users could set the ABI version to 1.4.3 (10403) and thus enable such "future" ABI features, and for instance setting it to 10499 would allow all future ABI changes...

Any opinions?

[Albrecht Schlosser]

On 3/28/25 21:57 Albrecht Schlosser wrote:

On 3/28/25 20:53 Greg Ercolano wrote:

    Regarding adding virtual, I'll handle this; expect a commit to 1.5.x today.

Thanks. FTR: Greg's commit is 9e1e043af35e979f5ad0824fb74f6e7fab57be7d

    As I believe Albrecht mentioned in a previous reply, he didn't think this should be
    added to 1.4.x as an ABI feature, so I'll leave that out.

This was my initial thought, but later, when I realized that it was such a trivial modification, I changed my mind.

Anyway, I can do the backport, particularly because an ABI guard against ABI version 10403 (1.4.3) would not work w/o changing the API version to 1.4.3 as well (see my previous message).

Backport done in commit 26d183ca778cceea85b2ba6547aeede64181cc64 ...

I'm thinking about changing the max-allowed ABI version to (API version + 1). This would allow to set the ABI version of the *next* release so ABI guards for that version would allow that code to be compiled. Then we could leave the API version at 1.4.2 (as long as it's not released), but users could set the ABI version to 1.4.3 (10403) and thus enable such "future" ABI features, ...

Done as well:

 - commit 6336959306e0540ef492d57d159f64ad1225cf93 in `branch-1.4` (1.4.2 + Git)
 - commit cb86a37676e26a8de209a371132d8154986b457a in `master` (1.5.0 in Git).

Now it is possible to set the ABI version in `branch-1.4` (currently 1.4.2) to `10403` to enable ABI features of the *next* release (1.4.3) and thus use the new ABI feature of Fl_Tree_Item (new virtual methods).

Enjoy!

[Rob McDonald]

Thanks a ton!

Rob

 

[Tim Cuatt]

Hi everyone,

Thanks all for your swift work. We really appreciate it over here! Apologies I wasn't replying to the thread- thanks Rob for replying on our behalf.

Best,

-Tim C