bugGNU Octave - Bugs: bug #63051, Reparented graphics objects may be...

 
 

bug #63051: Reparented graphics objects may be drawn in wrong order

Submitter:  Muhali <muhali>
Submitted:  Mon 12 Sep 2022 11:59:27 AM UTC
   
 
Category:  Plotting with OpenGL Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Confirmed Assigned to:  None
Originator Name:  Open/Closed:  * Open
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 09 Oct 2022 12:38:22 PM UTC, comment #12: 

This has already been happening in Octave 6.4.0. So, it is not a recent regression.
Updating release to "dev".

Markus Mützel <mmuetzel>
Group administrator
Tue 20 Sep 2022 10:24:43 PM UTC, comment #11: 

The test case in bug #55634 also passes with the set_zliminclude patch.  I don't know why this should be happening either.  Unfortunately, this may require using the debugger and stepping through the three drawing passes in gl-render.cc to see when each object is being drawn.

Rik <rik5>
Group administrator
Tue 20 Sep 2022 07:06:20 AM UTC, comment #10: 

Does the test case in bug #55634 still work with that change?

I'm still struggling to understand why any of this would affect the order in which objects are drawn by the gl-renderer.
Maybe lines in a hggroup are drawn at a different z-level? But then, why is it "fixed" when drawing an additional line on top?

Markus Mützel <mmuetzel>
Group administrator
Mon 19 Sep 2022 10:36:58 PM UTC, comment #9: 

Following up on my own comment #8, attached is a simple patch against the current development tip that allows the test case tst_reparent.m to pass.  All it does is restore the "set_zliminclude" call.

(file #53724, file #53725)

Rik <rik5>
Group administrator
Mon 19 Sep 2022 09:15:58 PM UTC, comment #8: 

I was surprised this changeset was the first indication of problems as well.  But, I did do "bootstrap && configure && make" on each changeset to make sure I was starting from scratch each time.

It may be that it it was the calling of set_zliminclude which effectively forced a repaint of the plot area which was getting things to work earlier.

From 26715 in graphics.in.h,


-    void update_zdata (void)
-    {
-      set_zlim (zdata.get_limits ());
-      set_zliminclude (get_zdata ().numel () > 0);
-    }
+    void update_zdata (void) { set_zlim (zdata.get_limits ()); }
   };


Rik <rik5>
Group administrator
Mon 19 Sep 2022 10:07:59 AM UTC, comment #7: 

Thanks for tracking this down.
I was half expecting that this might be caused by some changes when `light` was added. IIRC, I had to touch the order in which objects are drawn to get the light source early enough when rendering.

I don't understand immediately how the changeset that Rik found affects the order in which objects are drawn though.
IIUC, that drawing is done in `opengl_renderer::draw_axes_children`:
https://hg.savannah.gnu.org/hgweb/octave/file/6646f2b5a3d1/libinterp/corefcn/gl-render.cc#l2210

Lines (including those in hggroups) should be drawn in "group #3":

   // 3rd pass: draw remaining objects

    m_glfcns.glDisable (GL_DEPTH_TEST);

    for (it = obj_list.begin (); it != obj_list.end (); it++)
      {
        graphics_object go = (*it);

        set_clipping (go.get_properties ().is_clipping ());
        draw (go);
      }

    set_clipping (false);


`obj_list` is populated here:
https://hg.savannah.gnu.org/hgweb/octave/file/6646f2b5a3d1/libinterp/corefcn/gl-render.cc#l2163

  void
  opengl_renderer::draw_all_lights (const base_properties& props,
                                    std::list<graphics_object>& obj_list)
  {
#if defined (HAVE_OPENGL)
    gh_manager& gh_mgr = __get_gh_manager__ ();

    Matrix children = props.get_all_children ();

    for (octave_idx_type i = children.numel () - 1; i >= 0; i--)
      {
        graphics_object go = gh_mgr.get_object (children(i));

        base_properties p = go.get_properties ();

        if (p.is_visible ()
            || (m_selecting && p.pickableparts_is ("all")))
          {
            if (go.isa ("light") && ! m_selecting)
              {
                if (m_current_light-GL_LIGHT0 < m_max_lights)
                  {
                    set_clipping (p.is_clipping ());
                    draw (go);
                    m_current_light++;
                  }
              }
            else if (go.isa ("hggroup")
                     && ! (m_selecting && p.pickableparts_is ("none")))
              draw_all_lights (go.get_properties (), obj_list);
            else if (! (m_selecting && p.pickableparts_is ("none")))
              obj_list.push_back (go);
          }
      }
#else

    octave_unused_parameter (props);
    octave_unused_parameter (obj_list);

    // This shouldn't happen because construction of opengl_renderer
    // objects is supposed to be impossible if OpenGL is not available.

    panic_impossible ();

#endif
  }


I'm not sure how any of this can be affected by the changeset that Rik found...

Markus Mützel <mmuetzel>
Group administrator
Mon 19 Sep 2022 04:54:29 AM UTC, comment #6: 

Bisecting was very slow and difficult because it spanned the default and stable branches.  I finally narrowed it down to this changeset:


parent: 26715:24e11ee4fe45
 Improve calculation of zlim and aspect ratios for 2d objects (bug #55634).
branch: default



Rik <rik5>
Group administrator
Fri 16 Sep 2022 11:59:35 AM UTC, comment #5: 

@Rik: I have looked at this issue a bit and haven't been able to identify its root cause (lines are drawn in the same order in gl-render.cc but sorted differently back to front depending on the number of lines, very strange). AFAICS, running the same offending example using patch objects rather than lines doesn't trigger the bug:


clf; hold on
h1 = hggroup ();
h2 = patch ([0 1 0.5], [0 0 1], "r");
h3 = patch ([0 1 0.5], [1 1 0], "b", "parent", h1);


and the blue patch is drawn below the red one, as expected...

So in the end, bisecting seems difficult (we'll have to inspect visually each resulting figure) but necessary.

Pantxo Diribarne <pantxo>
Group Member
Fri 16 Sep 2022 12:48:35 AM UTC, comment #4: 

I tested backwards through a few versions of Octave and it is broken in 6.4 but works in 5.2.  I don't know whether it is worth trying to use "hg bisect" or just to go straight to graphics.cc to find the problem.

Rik <rik5>
Group administrator
Fri 16 Sep 2022 12:45:50 AM UTC, comment #3: 

Confirmed with Pantxo's example.

Rik <rik5>
Group administrator
Mon 12 Sep 2022 01:47:13 PM UTC, comment #2: 


comment #1:

> If you agree, I'll change the title to something like "Reparented graphics objects may be drawn in wrong order"


of course, go ahead!

Muhali <muhali>
Mon 12 Sep 2022 01:04:01 PM UTC, comment #1: 

The problem is not the addition of the legend, which I could verify doesn't touch the axes children stack.
The red dot should be draw below the blue line right from the beginning since it is a children of the hggroup which is supposed to be drawn first. The bug is that until another child is added to the axes (what legend silently does), the order in which graphics objects are drawn may be wrong. Here is a script to demonstrate the issue:


clf;
hold on;
h1 = hggroup ();
h2 = plot (1:3);
plot (2, 2,"or", "parent", h1, "markerfacecolor", "r", "markersize", 50)
h3 = plot (3:-1:1, "-g");


Before drawing h3, the red dot is above, while after it is below other objects, as it should.

If you agree, I'll change the title to something like "Reparented graphics objects may be drawn in wrong order"

Pantxo Diribarne <pantxo>
Group Member
Mon 12 Sep 2022 11:59:27 AM UTC, original submission:  

doing the following


y = 1:3 ;
clf ; hold on
h = hggroup ;
plot(y) ;
plot(2, 2, "o", "parent", h, "markerfacecolor", "r", "markersize", 50) ;
legend(h, {"blob"})


one sees that the blob has been put behind the line by the legend command.

Muhali <muhali>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53724:  zliminclude.patch added by rik5 (785B - text/x-patch)
file #53725:  tst_reparent.m added by rik5 (157B - text/x-matlab)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by muhali (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-10-09 mmuetzel Release7.2.0 dev
    2022-09-20 mmuetzel Release7.1.0 7.2.0
        Operating SystemGNU/Linux Any
    2022-09-19 rik5 Attached File- Added zliminclude.patch, #53724
        Attached File- Added tst_reparent.m, #53725
    2022-09-16 rik5 Item GroupNone Incorrect Result
    2022-09-12 pantxo Summarylegend modifies graphics objects stack Reparented graphics objects may be drawn in wrong order
    2022-09-12 pantxo StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code