bugGNU Octave - Bugs: bug #58508, scatter: face color ignored when...

 
 

bug #58508: scatter: face color ignored when filled is given

Submitter:  Hg200 <hg200>
Submitted:  Sat 06 Jun 2020 01:59:04 PM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  hg200 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

Sun 07 Jun 2020 12:27:35 PM UTC, comment #8: 

Thanks for confirming the fix.

Properties are set from left to right. So if a property is set twice, the rightmost one is the one that is used in the end.

Markus Mützel <mmuetzel>
Group administrator
Sun 07 Jun 2020 12:08:09 PM UTC, comment #7: 

The test results are fine now.

Yes i agree it is better not to set the default values ​​twice (_scatter_.m and graphics.in.h) if the 'filled' option is not specified.

However, I could not find out in which order the properties are parsed (probably somewhere in graphics.cc) and whether the argument on the right overrides a one on the left. Because if 'fill' is specified we send the property 'markerfacecolor' twice to _go_scatter_(). But as said, the test plots are correct so i think it is okay.

filled_args{:}
ans = markeredgecolor
ans = none
ans = markerfacecolor
ans = flat

newargs{:}
ans = markerfacecolor
ans = r

Hg200 <hg200>
Sun 07 Jun 2020 08:37:06 AM UTC, comment #6: 

I think we should set the default property values in a single place only (i.e. in graphics.in.h).
I fixed that in the following changeset:
https://hg.savannah.gnu.org/hgweb/octave/rev/9cf726d43735

I believe that should also fix the other cases here.

Marking as ready for test again.

Markus Mützel <mmuetzel>
Group administrator
Sun 07 Jun 2020 07:48:58 AM UTC, comment #5: 

Pretty close but the patch returns 'markeredgecolor' as 'flat' instead of 'none' for comment #1.

If you replace 'fill_args' with 'filled_args', _go_scatter_ is set up correctly and everything is fine.

Hg200 <hg200>
Sun 07 Jun 2020 04:41:07 AM UTC, comment #4: 

This was actually slightly more complicated than it appeared.  The scatter graphics object property "markeredgecolor" needed to implement the value "none".  Matlab does not reference this value in their documentation, but it exists, and can be checked by running code in Matlab.

I checked in a change for that here: https://hg.savannah.gnu.org/hgweb/octave/rev/04349a21c750.

Now, example #1 in comment #3 correctly sets "markerfacecolor" to "flat" and "markeredgecolor" to "none".  The case in example #2 is also corrected.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Sat 06 Jun 2020 09:44:35 PM UTC, comment #3: 

Thanks for the short-term fix. A comparison between Matlab r2018 and the fix 6f415e9b14ae yields following result:

i.)
scatter(...,"filled")
->
In r2018 and also according to mathworks documentation, the markerfacecolor is set to flat and the markeredgecolor to none. With the fix 6f415e9b14ae, however, the face is set to flat and the edge to flat.

ii.)
scatter(...,"filled","markerfacecolor","r")
->
In r2018 a patch is plot according to the face color red, but without an edge (edge is set to none). With the fix 6f415e9b14ae the face is set to red and the edge to flat.

iii.)
scatter(...,"filled","markerfacecolor","markeredgecolor")
->
Draws a patch according to the specified face color and an edge according to the specified edge color. In fix 6f415e9b14ae this is displayed correctly.

I ran some more tests and found no additional issues compared to Matlab r2018. The new scatter code looks very good. I have attached the test cases for reference.


(file #49221)

Hg200 <hg200>
Sat 06 Jun 2020 08:56:50 PM UTC, comment #2: 

Confirmed.  As another example of the inconsistency, this syntax works


scatter (xx, yy, 'r', 'filled');



Rik <rik5>
Group administrator
Sat 06 Jun 2020 06:56:36 PM UTC, comment #1: 

Thanks for reporting this regression.
I pushed a change here that should fix it on default:
http://hg.savannah.gnu.org/hgweb/octave/rev/6f415e9b14ae

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Sat 06 Jun 2020 01:59:04 PM UTC, original submission:  

A small issue regarding Matlab compatibility. The argument 'filled' overrides the setting of the face color so that patch elements are shown flat instead of red.

If we run the following code in Matlab r2018 or Octave 5.2.0, patches are plot red. On the devel branch they are plot flat.


xx = [1 2 3 3 3 1 1];
yy = [1 2 3 1 3 1 3];
scatter (xx, yy, 'filled', 'markerfacecolor', 'r');



Hg200 <hg200>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49221:  scatterbenchmark.m added by hg200 (2KiB - text/x-objcsrc)

 

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 mmuetzel (Posted a comment)
  • -email is unavailable- added by hg200 (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 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-07 mmuetzel StatusReady For Test Fixed
    2020-06-07 mmuetzel StatusFixed Ready For Test
    2020-06-07 rik5 CategoryPlotting Plotting with OpenGL
        StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2020-06-06 hg200 Attached File- Added scatterbenchmark.m, #49221
    2020-06-06 rik5 CategoryPlotting with OpenGL Plotting
        Item GroupRegression Matlab Compatibility
        StatusReady For Test Confirmed
    2020-06-06 mmuetzel CategoryPlotting Plotting with OpenGL
        Item GroupMatlab Compatibility Regression
        StatusNone Ready For Test

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code