taskGNU Octave - Tasks: task #14243, Rewrite legend.m

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #14243: Rewrite legend.m

Submitted by:  Pantxo Diribarne <pantxo>
Submitted on:  Thu 17 Nov 2016 10:11:23 AM UTC  
Votes:  21  
 
Should Start On: Thu 17 Nov 2016 12:00:00 AM UTCShould be Finished on: Thu 17 Nov 2016 12:00:00 AM UTC
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOriginator Name: 
Open/Closed: OpenFixed Release: None
Planned Release: None

(Jump to the original submission Jump to the original submission)

Tue 26 Dec 2017 11:58:06 PM UTC, comment #7:

I eliminated the questionable use of the "userdata" property to hold internally private legend data in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/19d63669d174).

Rik <rik5>
Project Administrator
Thu 21 Dec 2017 10:04:18 PM UTC, comment #6:

OK, a lot of work, but I think any new minor bugs are worth the change.

This conditional:

All it is really doing is encouraging users to file a bug report. It creates an error with regard to Octave's internal programming, not something caused by the user (like an ill-formed matrix, etc.). But without this it will result in an error eventually also via using a bogus handle. So, I don't know how beneficial this was in the first place. If it is possible the user inputs a bogus handle, then it is O.E. and we don't want a bug report for that.

Missed line-space here?:

Dan Sebald <sebald>
Thu 21 Dec 2017 12:21:48 PM UTC, comment #5:

Not that it is relevant for this task, but I significantly rewrote legend.m for greater Matlab compatibility in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/436deae9f9ab).

Rik <rik5>
Project Administrator
Wed 13 Dec 2017 08:15:07 PM UTC, comment #4:

Investigating bug #52641 led me to look more closely at legend.m. My suggestion is to make this more listener/callback oriented and get rid of some of the old-convention techniques like persistent variables, etc. If one gets in the listener/callback frame of mind it becomes much easier to realize simpler ways of doing things.

1) Drop the use of "userdata" everywhere. Not only is it legend.m where this "userdata" variable is being used to store things. "UserData" is for the user to use as desired. Even if some of these objects typically don't have a UserData counterpart in Matlab, if the user decides to use some legend "userdata" it will disrupt the legend.

2) Make the listeners more abundant and at the same time smaller hunks. That is the typical strategy of callback-based frameworks. The behavior lies in all the connections made. For example, rather than reconstructing the legend via calling legend() again and again, there could be listeners tied to the axes children list. If the children list changes (due to addition of another plot item, e.g., "hold on") it could trigger a reconstruction of the plot. Listeners could be tied to the name text objects such that if one of those text objects changes--either directly or via "set(hax, 'displayname', 'newname')" which in itself triggers some callbacks--it will trigger a re-layout of the plot.

3) The way that the current-axes and legend-handle are derived seems far more complex than it needs to be. If the input is a valid axes handle, use that, otherwise gca() does the job, even constructing the axes if they don't exist. All that searching of the ancestor tree should be replaced by something far more simple.

4) With regard to the reoccurring scanning of ancestors and kids, it might be better to place more information in the list of arguments to listeners. It makes the number of variables passed around greater, but at the same time it saves running a hunk of code to reconstruct that info.

Dan Sebald <sebald>
Wed 23 Nov 2016 11:03:45 AM UTC, comment #3:

Here are a few other tests inspired by the current demos

Pantxo Diribarne <pantxo>
Project Member
Thu 17 Nov 2016 09:53:54 PM UTC, comment #2:

I think bug #49446 boils down to implementing the selection tool in Qt/Fltk toolkits but indeed legend objects should be able to handle custom positioning properly.

As a starter I'll dump here some test scripts (written ML compatible for testing) that should pass and that currently pass or not with Octave's implementation.

Pantxo Diribarne <pantxo>
Project Member
Thu 17 Nov 2016 06:59:06 PM UTC, comment #1:

Can bug #49446 added to the list, please? (at least for legends)

Philip Nienhuis <philipnienhuis>
Project Member
Thu 17 Nov 2016 10:11:23 AM UTC, original submission:

There are a couple of bug reports (that partly overlap) that show the need for (complete?) rewrite of legend.m:

  • bug #49940, bug #41142: related to the fact that legend deletes and recreates text objects when someone tries to change a single text property on the object directly
  • bug #39697, bug #39316: related to the legend object not having listeners on the figure/axes position so that it doesn't behave well when the figure or axes rescaled
  • bug #47455: building a legend is slow, updating it is also very slow since most of the time the legend is entirely rebuilt.
  • bug #40333: changing the fontsize on a legend modifies its corresponding axes position, if the latter position has changed since the legend was created
Pantxo Diribarne <pantxo>
Project Member

 

No files currently attached

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by sebald (Posted a comment)
  • -unavailable- added by johasixt (Voted in favor of this item)
  • -unavailable- added by reginalduck (Voted in favor of this item)
  • -unavailable- added by pantxo (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 21 encouragements so far.

    Only logged-in users can vote.

     

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

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 08 Feb 2017 12:10:02 PM UTCjohasixtCarbon-Copy-=>Added johasixt
    Wed 30 Nov 2016 02:10:24 PM UTCreginalduckCarbon-Copy-=>Added reginalduck
    Fri 18 Nov 2016 06:26:34 AM UTCsiko1056Dependencies-=>Depends on bugs #40333

    Back to the top


    Powered by Savane 3.1-cleanup1