bugGNU Octave - Bugs: bug #52621, root graphics object property...

 
 

bug #52621: root graphics object property "currentfigure" does not respect figure "handlevisibility" property

Submitted by:  jsh <jsh>
Submitted on:  Fri 08 Dec 2017 02:41:09 AM UTC  
 
Category: PlottingSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Open/Closed: Closed
Release: devOperating System: Any

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 Jump to the original submission)

Mon 18 Dec 2017 07:12:12 PM UTC, comment #12:

I changed the remaining instances of gcbf() where there might be problems with the new behavior. Most of the time the code could just be deleted as it was unnecessary, so we get a small performance boost too!

Rik <rik5>
Project Administrator
Thu 14 Dec 2017 12:28:04 AM UTC, comment #11:

I changed Octave's mechanisms around close() and closereq() functions to match Matlab's behavior. See cset http://hg.savannah.gnu.org/hgweb/octave/rev/9213b8166af9.

Rik <rik5>
Project Administrator
Tue 12 Dec 2017 12:13:13 AM UTC, comment #10:

Looking at closereq.m, we would need to slow things down and use a for loop in the m-file if we want to implement things exactly like Matlab.

Currently, the code just collects the figures to process and then runs

If every figure to be closed should be the gcf then this would need to be modified to

Actually, since figure.m is a reasonably complex m-file, we might want to shorten that up by just using

Rik <rik5>
Project Administrator
Mon 11 Dec 2017 11:49:32 PM UTC, comment #9:

@Pantxo: Interesting. In the documentation I see

So it is the case that closereq doesn't delete HiddenHandle figures, but if "clase all hidden" is used it must delete these figures too. It sounds like 'close all hidden' changes the HandleVisibility property.

One other thing, could you test this in Matlab?

Does it issue a warning?

Rik <rik5>
Project Administrator
Mon 11 Dec 2017 10:57:51 PM UTC, comment #8:

@Rik: sorry, I had seen those warning during tests but hadn't had time to look at them.

  • closereq: according to the ML doc for the "closerequestfcn" property [1], "close" should make figure current before evaluating the closerequestfcn so that closereq can rely on gcf (instead of gcbf) to call the "delete" function.
  • others: I had no time to look at them but I am under the impression that we can workaround the possibly empty gcbf using "ancestor (hax, 'figure')".

[1] https://fr.mathworks.com/help/matlab/ref/matlab.ui.figure-properties.html#buiwuyk-1-CloseRequestFcn

Pantxo Diribarne <pantxo>
Project Member
Mon 11 Dec 2017 05:23:32 PM UTC, comment #7:

@Pantxo: Good work. I checked in the patch here http://hg.savannah.gnu.org/hgweb/octave/rev/41cf6ee90cb6.

This is enough of a change that odd side effects are likely to crop up for a while. For example, I tried this.

The problem is in closereq.m

The file closereq.m is trying to use gcbf to see whether a callback is executing. It seems like either we could get rid of the warning (which still wouldn't work for hidden figures), we could modify close.m to reset the handlevisibility property of all objects that it is about to call the closerequestfcn, or we have to invent something in C++ graphics code that allows gcbf to show through for figures which are being deleted (maybe when the "beingdeleted" property is true?).

I looked through the m-files for gco, gcbo, gcbf instances which might cause problems. The only ones that looked like they might be an issue were these from gcbf

I haven't reviewed any of the other instances besides closereq.m which I stumbled upon.

Rik <rik5>
Project Administrator
Sun 10 Dec 2017 01:44:23 PM UTC, comment #6:

@Rik: As suggested, I attached an updated cset that handles currentobject, currentaxes and callbackobject as well (plus corresponding BIST).

(file #42610)

Pantxo Diribarne <pantxo>
Project Member
Fri 08 Dec 2017 04:47:36 PM UTC, comment #5:

@Pantxo: The patch works very well for the issue at hand. Looking at the documentation for the Figure "handlevisibility" property for Matlab

In the third paragraph Octave now implements the first two conditions

1) object does not appear in parent object's "children" property
2) figures do not appear in root object's "CurrentFigure" property

Would it be possible for you to code a solution for the next two conditions as well?

3) object does not appear in root "CallbackObject" property (which gco() uses)
4) axes objects do not appear in their parent's "CurrentAxes" property.

As test code for the last condition,

when I run that today I get

Rik <rik5>
Project Administrator
Fri 08 Dec 2017 10:57:45 AM UTC, comment #4:

I attached a patch that makes Octave behave as ML with regards to Rik's test. With this patch, the following test (not included in Rik's) returns "2" followed by "1":

This means changing the visibility of the currentfigure not only makes it "not current" but also sets the first existing-visible-figure in the stack "current".

(file #42605)

Pantxo Diribarne <pantxo>
Project Member
Fri 08 Dec 2017 09:36:38 AM UTC, comment #3:

In Matlab R2016a:

Markus Mützel <mmuetzel>
Project Member
Fri 08 Dec 2017 06:26:40 AM UTC, comment #2:

It would be useful if someone with access to Matlab could run the following code and report back the results.

Rik <rik5>
Project Administrator
Fri 08 Dec 2017 06:22:25 AM UTC, comment #1:

Confirmed. Although the details are a little bit intricate and I have retitled this bug report to reflect that.

Octave does implement handlevisibility as shown below.

The problem is that while the figure is removed from the list of children, it is not necessarily removed from the "currentfigure" property.

I can get your example code to work by forcing the "currentfigure" property to be empty.

Rik <rik5>
Project Administrator
Fri 08 Dec 2017 02:41:09 AM UTC, original submission:

figure property handlevisibility is not respected (at least not with the Qt tookit on Octave Windows builds 4.0.3 and 4.2.1)

The following code snippet should open a new figure to plot in, instead the plot is drawn in the "invisible" figure.

jsh <jsh>

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #42610:  fig_visib2.patch added by pantxo (7KiB - text/x-patch)
file #42605:  fig_visib.patch added by pantxo (3KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pantxo (Updated the item)
  • -unavailable- added by mmuetzel (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by jsh (Submitted the item)
  •  

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

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 10 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 18 Dec 2017 07:12:12 PM UTCrik5StatusPatch Reviewed=>Fixed
      Open/ClosedOpen=>Closed
    Mon 11 Dec 2017 05:23:32 PM UTCrik5StatusPatch Submitted=>Patch Reviewed
      Release4.2.1=>dev
    Sun 10 Dec 2017 01:44:23 PM UTCpantxoAttached File-=>Added fig_visib2.patch, #42610
    Fri 08 Dec 2017 10:57:45 AM UTCpantxoAttached File-=>Added fig_visib.patch, #42605
      StatusConfirmed=>Patch Submitted
    Fri 08 Dec 2017 06:22:25 AM UTCrik5StatusNone=>Confirmed
      Operating SystemMicrosoft Windows=>Any
      Summaryfigure property handlevisibility is not respected=>root graphics object property "currentfigure" does not respect figure "handlevisibility" property

    Back to the top


    Powered by Savane 3.1-cleanup1