bugGNU Octave - Bugs: bug #44801, Removing a context menu

 
 

bug #44801: Removing a context menu

Submitted by:  Guillaume <gyom>
Submitted on:  Fri 10 Apr 2015 02:51:08 PM UTC  
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Guillaume Open/Closed:  Closed
Release:  dev Operating System:  GNU/Linux

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 14 Apr 2015 09:41:09 PM UTC, comment #16:

I pushed the patch here:
http://hg.savannah.gnu.org/hgweb/octave/rev/69f92e0affd2

Closing report.

Pantxo Diribarne <pantxo>
Project Member
Tue 14 Apr 2015 02:01:38 AM UTC, comment #15:

Go ahead and push it. The ui* series of functions need a lot of work and it would be nice if we could update them with bug fixes, rather than have to wait until 4.2 because we don't want to break ABI compatibility.

Rik <rik5>
Project Administrator
Sun 12 Apr 2015 09:43:06 AM UTC, comment #14:

So should I push it now or wait for 4.2? I am pretty sure it's innocuous for everything else than uicontextmenu related operations.

Pantxo Diribarne <pantxo>
Project Member
Sat 11 Apr 2015 05:11:17 PM UTC, comment #13:

Good point. We don't want to break ABI compatibility.

Rik <rik5>
Project Administrator
Sat 11 Apr 2015 04:05:01 PM UTC, comment #12:

Well, I am not sure this patch is a good candidate for minor release as it adds a public method to the uicontextmenu class. This will break ABI compatibility and AFAIU it's not good practice for minor releases.

Pantxo Diribarne <pantxo>
Project Member
Sat 11 Apr 2015 03:58:31 PM UTC, comment #11:

@Panxto: Maybe hold off until after the 4.0.0 release. We're anticipating that there will be a very quick 4.0.1 release 6-8 weeks after 4.0.0 to deal with whatever is uncovered by wide scale testing. Maybe it can go in then.

Rik <rik5>
Project Administrator
Sat 11 Apr 2015 02:54:23 PM UTC, comment #10:

@Rik: I think the patch is armless but as you are in the process of releasing 4.0 it's up to you wether I should push it now? If so can I add documentation for "uicontextmenu" property or did you already start grammar/spell-check task?

Pantxo Diribarne <pantxo>
Project Member
Sat 11 Apr 2015 02:47:22 PM UTC, comment #9:

Also works for me.

Rik <rik5>
Project Administrator
Sat 11 Apr 2015 02:29:18 PM UTC, comment #8:

Tested as well, works as expected. Thanks!

Guillaume <gyom>
Sat 11 Apr 2015 01:52:01 PM UTC, comment #7:

I have attached a more general fix which includes BIST tests:

  • setting "uicontextmenu" to [] only resets the value
  • deleting a uicontextmenu object will also reset the "uicontextmenu" property of all the objects that have it as current.

(file #33639)

Pantxo Diribarne <pantxo>
Project Member
Sat 11 Apr 2015 12:35:05 PM UTC, comment #6:

It works for me - many thanks @pantxo!

Guillaume <gyom>
Sat 11 Apr 2015 11:26:24 AM UTC, comment #5:

I have attached a simple patch that fixes the issue for me. However it does only handle the case of deleting a uicontextmenu and reseting its parent "uicontextmenu" property.
I am working on a more general solution that uses Rik's suggestion to do the cleanup in the uicontextmenu destructor. uicontextmenu objects will store a list of objects that may depend on them, and reset their "uicontextmenu" property at destruction if needed.

(file #33637)

Pantxo Diribarne <pantxo>
Project Member
Sat 11 Apr 2015 11:12:42 AM UTC, comment #4:

Thanks for looking into this - I was especially not aware of the usage of NaN in this particular setting.

I was also at first thinking that "set(F,'uicontextmenu',[])" was not a thing to do but, actually, this is and should be valid and has a useful role if you don't want to delete a uicontextmenu but just remove it temporarily and reuse it later on:

Unfortunately NaN works with Octave and not MATLAB, while [] works with MATLAB and not Octave.

Also flagging that there might be a link between this discussion and bug #44763.

Guillaume <gyom>
Sat 11 Apr 2015 07:49:18 AM UTC, comment #3:

I am working on a simple solution that doesn't involve making use of the deletefcn (which we should keep available for the user).

Pantxo Diribarne <pantxo>
Project Member
Fri 10 Apr 2015 11:02:37 PM UTC, comment #2:

It appears that Matlab is also not that good about cleaning up after uicontextmenu's and requires the user to do it themselves (http://undocumentedmatlab.com/blog/uicontextmenu-performance).

Rik <rik5>
Project Administrator
Fri 10 Apr 2015 10:33:12 PM UTC, comment #1:

The problem appears to be that the C++ graphics code assumes that whenever an object is deleted it should be removed from it's parent's "children" property. In this case, the graphics handle for the menu is under the "uicontextmenu" rather than "children.

You can hack around this for now, or dive into the source if it is really bothersome.

First, delete() is the correct thing to do to remove a handle so

will rid Octave of the uicontextmenu. The 'uicontextmenu' property expects a graphics_handle to be entered. This is a double value, where NaN has the special meaning of invalid. At some point, libinterp/corefcn/graphics.in.h should be modified to accept an empty matrix input ([]). For now, this will safely rid the figure of the now deleted uicontextmenu.

As a workaround that combines these two things you could write a special deletefcn to be called whenever a uicontextmenu is deleted.

Now you can write

Since uicontextmenu is an m-file in scripts/gui this could be made a default part of Octave. Or the C++ deletion code in graphics.in.h could be improved. The destructor ~uicontextmenu is currently empty. Maybe that is the place to check the parent and see if there is a 'uicontextmenu' property and delete it, rather than in an m-file callback.

Rik <rik5>
Project Administrator
Fri 10 Apr 2015 02:51:08 PM UTC, original submission:

There doesn't seem to be a clean way to remove a context menu from a figure:

Guillaume <gyom>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #33639:  fix_deletectx5.patch added by pantxo (6KiB - application/x-download)
file #33637:  fix_deletectx2.patch added by pantxo (2KiB - application/x-download)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by gyom (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-04-14 pantxo StatusIn Progress => Fixed
        Open/ClosedOpen => Closed
    2015-04-12 pantxo CategoryPlotting => Plotting with OpenGL
    2015-04-11 rik5 StatusConfirmed => In Progress
    2015-04-11 pantxo Attached File- => Added fix_deletectx5.patch, #33639
    2015-04-11 pantxo Attached File- => Added fix_deletectx2.patch, #33637
    2015-04-10 rik5 StatusNone => Confirmed

    Back to the top


    Powered by Savane 3.4