bugGNU Octave - Bugs: bug #52795, "pickableparts"...

 
 

bug #52795: "pickableparts" unimplemented

Submitter:  Pantxo Diribarne <pantxo>
Submitted:  Wed 03 Jan 2018 02:35:42 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 12 Feb 2018 10:45:11 PM UTC, comment #10: 

Checked in here (http://hg.savannah.gnu.org/hgweb/octave/rev/f7e333b12687).  Thanks Pantxo for single-handedly implementing this property.  Closing report as fixed.

Rik <rik5>
Group administrator
Mon 12 Feb 2018 04:27:08 PM UTC, comment #9: 

@Rik: I checked in ML today :

  • the figure "buttondownfcn" and "windowbuttondown(up)fcn" are always executed, whatever the value of the "hittest" property. The attached patch fixes this for "buttondownfcn", others are already OK.
  • uicontextmenu are not executed if "hittest" is "off" (that is already the case in Octave)



(file #43262)

Pantxo Diribarne <pantxo>
Group Member
Wed 31 Jan 2018 12:54:05 AM UTC, comment #8: 

@Pantxo: Could you check out the final question at the bottom of comment #7 which is how Octave is implementing HitTest for figures?  I feel we are very close to finishing this bug report.

Rik <rik5>
Group administrator
Wed 24 Jan 2018 01:45:13 AM UTC, comment #7: 

@Pantxo: I checked in your 5th changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/9d01ce02d5cb).

One small optimization I made was to look up the parent directly rather than going through the double value which represents the graphic handle.  The function get_parent () returns a graphics_handle which get_object can use directly.  Otherwise, get_object() has to call lookup() first on the double value passed to it in order to get a graphics_handle object and can then call the internal do_get_object() function.


diff -r b5c731136924 libgui/graphics/Canvas.cc
--- a/libgui/graphics/Canvas.cc        Mon Jan 22 16:46:59 2018 +0100
+++ b/libgui/graphics/Canvas.cc        Tue Jan 23 17:24:17 2018 -0800
@@ -640,12 +640,10 @@ namespace QtHandles
           {
             // Objects with "hittest"->"off" pass the mouse event to their
             // parent and so on.
-            graphics_object tmpgo
-              = gh_manager::get_object (currentObj.get ("parent").
-                                        double_value ());
+            graphics_object tmpgo;
+            tmpgo = gh_manager::get_object (currentObj.get_parent ());
             while (tmpgo && ! tmpgo.get_properties ().is_hittest ())
-              tmpgo = gh_manager::get_object (tmpgo.get ("parent")
-                                              .double_value ());
+              tmpgo = gh_manager::get_object (tmpgo.get_parent ());

             if (tmpgo && tmpgo.get_handle () != 0.0)
               currentObj = tmpgo;


There still might be some weirdness with respect to the HitTest property and Figure objects.  As I understand this documentation, Figures ignore the HitTest property when it comes to executing callback functions.


Figures do not have a PickableParts property. Figures execute button callback functions regardless of the setting of their HitTest property.


But this is not what Octave does.  If I use your test code from comment #5


fcn = @(h,e) disp (get (h, 'type'));
fcn2 = @(h,e) disp ([get(h, 'type') '2']);
hp1 = patch ('buttondownfcn', fcn);
hg = hggroup ('buttondownfcn', fcn);
hp2 = patch ('parent', hg, 'buttondownfcn', fcn2);
set (gca, 'buttondownfcn', fcn);
set (gcf, 'buttondownfcn', fcn);
## click on the patch -> "patch2"
set (hp2, 'hittest', 'off')
## click on the patch -> "hggroup"
set (hg, 'hittest', 'off')
## click on the patch -> "axes"
set (gca, 'hittest', 'off')
## click on the patch -> "figure"


then everything works as expected.  Now if I add


set (gcf, 'hittest', 'off')


then clicks on the figure itself, outside the axes object, cause "figure" to be displayed.  But clicking on the patch object no longer causes "figure" to be printed.

If I check the HitTest property for figures the documentation is


HitTest — Ability to become current object
'on' (default) | 'off'

Ability to become current object, specified as 'on' or 'off':

    Setting the value to 'on' allows the Figure object to become the current object when the user clicks on the corresponding UI component. A value of 'on' also allows the figure's CurrentObject property and the gco function to report the Figure as the current object.

    Setting the value to 'off' sets the figure's CurrentObject property to an empty GraphicsPlaceholder array when the user clicks on the UI component.


It appears that "HitTest" still means something for Figures, the ability to become the CurrentObject or not, but it should have no influence on whether callbacks are run.

Rik <rik5>
Group administrator
Mon 22 Jan 2018 03:52:45 PM UTC, comment #6: 

Here is a new iteration of the patch that handles the case when the ancestor figure's "hittest" is also "off" (gco == [], as stated here https://fr.mathworks.com/help/matlab/ref/matlab.ui.figure-properties.html#property_d119e285631).

(file #43020)

Pantxo Diribarne <pantxo>
Group Member
Mon 22 Jan 2018 01:48:01 PM UTC, comment #5: 

Today at work I tested how "hittest" works:


fcn = @(h,e) disp (get (h, 'type'));
fcn2 = @(h,e) disp ([get(h, 'type') '2']);
hp1 = patch ('buttondownfcn', fcn);
hg = hggroup ('buttondownfcn', fcn);
hp2 = patch ('parent', hg, 'buttondownfcn', fcn2);
set (gca, 'buttondownfcn', fcn);
set (gcf, 'buttondownfcn', fcn);
## click on the patch -> "patch2"
set (hp2, 'hittest', 'off')
## click on the patch -> "hggroup"
set (hg, 'hittest', 'off')
## click on the patch -> "axes"
set (gca, 'hittest', 'off')
## click on the patch -> "figure"


This is the same behavior as what I obtain with the attached (fixed "||" -> "&&") patch.

(file #43018)

Pantxo Diribarne <pantxo>
Group Member
Sun 21 Jan 2018 10:29:21 PM UTC, comment #4: 

The fix was easier than expected. I attached a patch that addresses the last point in comment #2

(file #43009)

Pantxo Diribarne <pantxo>
Group Member
Sun 21 Jan 2018 09:26:43 PM UTC, comment #3: 


>> for absolute Matlab compatibility, not all objects accept the "all" value. ...


Yes it is much easier to have "pickableparts" a base_property so that you can access it through graphics_object::get_properties (), and anyway root and figure objects should not have this property at all so I chose the "unused property/value" route.

>> One thing I noticed is that even though the Patch object is on top of the Line object, it is always the Line object callback that is triggered ...


Confirmed, I have no clue

>> ... For example, when PickableParts is set to "none" on the axes object, I still think you should be able to interact with graphic objects within the axes.


No, it is clearly stated here [1] (see the second note) that if an axes has its "pickableparts"->"none", its children are not clickable.

>> When HitTest is "off", is the mouse click passed to the ancestor of the object which received the click (this is what the Matlab documentation says, but I'm not sure exactly what it means), or does the object which is behind the object which received the click get it instead? The latter is what is implemented, but I don't think that is right.


Yes, I have probably incorrectly interpreted the fact that when clicking on non visible parts of an object that has "pickableparts"->"visible", the click can be captured by objects below it([2]). But if an click is not captured by an object because its "hittest" is "off", then it is its ancestor that should inherit the click event. I'll look into this when I get some time.

[1] https://de.mathworks.com/help/matlab/creating_plots/capturing-mouse-clicks.html
[2] https://fr.mathworks.com/help/matlab/ref/matlab.graphics.primitive.patch-properties.html#property_d119e755263

Pantxo Diribarne <pantxo>
Group Member
Thu 04 Jan 2018 11:57:28 PM UTC, comment #2: 

@Pantxo: I pushed your patch here ().  I re-worded the documentation in genpropdoc.m slightly to try and make clearer the distinction between "pickableparts" and "hittest", although it's already not very clear with Matlab.

I had a few questions.  First, for absolute Matlab compatibility, not all objects accept the "all" value.  That is why I had the base property contain the only two universal values "visible" and "none" and I overrode the property on the individual objects which do support "all".  If it makes for an easier implementation then we can just keep things the way they are with the base property having all three values.

Second, I did some testing with the following script tst_pick.m


close all
hax = gca;
hl = plot ([0 1], [1 0], 'r', 'linewidth', 10);
hp = patch;

function cb (h, ~, msg)
  fprintf (2, 'Callback %s executed\n', msg);
  fflush (2);
endfunction

set (hax, 'buttondownfcn', {@cb, 'Axes'});
set (hl, 'buttondownfcn', {@cb, 'Line'});
set (hp, 'buttondownfcn', {@cb, 'Patch'});


One thing I noticed is that even though the Patch object is on top of the Line object, it is always the Line object callback that is triggered.  Of course, this happened before PickableParts was implemented so it is unrelated to your change; it's still wrong though.

It might be useful to get confirmation from someone with access to Matlab, but I think "PickableParts" only acts on a per object level and doesn't necessarily affect children.  For example, when PickableParts is set to "none" on the axes object, I still think you should be able to interact with graphic objects within the axes.

Finally, I'm not sure our implementation of HitTest is correct.  When HitTest is "off", is the mouse click passed to the ancestor of the object which received the click (this is what the Matlab documentation says, but I'm not sure exactly what it means), or does the object which is behind the object which received the click get it instead?  The latter is what is implemented, but I don't think that is right.

A simple test, after using tst_pick.m above, is


hg = hggroup;
set (hg, 'buttondownfcn', {@cb, 'HgGroup'});
set (hp, 'parent', hg);
set (hp, 'HitTest', 'off');
%% Now click on patch area.  Is HgGroup or Axes callback executed?



Rik <rik5>
Group administrator
Wed 03 Jan 2018 08:25:25 PM UTC, comment #1: 

I attached an updated patch with which the doc now building cleanly.

(file #42814)

Pantxo Diribarne <pantxo>
Group Member
Wed 03 Jan 2018 02:35:42 PM UTC, original submission:  

I attached a tentative implementation of the "pickableparts" property. Could someone who knkows how it is supposed to work in Matlab test it?

Pantxo Diribarne <pantxo>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43262:  btndown.patch added by pantxo (2KiB - text/x-patch)
file #43020:  pickableparts5.patch added by pantxo (3KiB - text/x-patch)
file #43018:  pickableparts4.patch added by pantxo (2KiB - text/x-patch)
file #43009:  pickableparts3.patch added by pantxo (2KiB - text/x-patch)
file #42838:  tst_pick.m added by rik5 (307B - text/x-matlab)
file #42814:  pickableparts2.patch added by pantxo (23KiB - text/x-patch)
file #42806:  pickableparts.patch added by pantxo (23KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-02-12 rik5 StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2018-02-12 pantxo Attached File- Added btndown.patch, #43262
    2018-01-22 pantxo Attached File- Added pickableparts5.patch, #43020
    2018-01-22 pantxo Attached File- Added pickableparts4.patch, #43018
    2018-01-21 pantxo Attached File- Added pickableparts3.patch, #43009
    2018-01-04 rik5 Attached File- Added tst_pick.m, #42838
    2018-01-04 rik5 StatusPatch Submitted In Progress
    2018-01-03 pantxo Attached File- Added pickableparts2.patch, #42814
        StatusNone Patch Submitted
    2018-01-03 pantxo Attached File- Added pickableparts.patch, #42806

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code