Re: [Plplot-devel] Infinite Yielding issue

[Pedro V. <ped...@sp...>]

Hi Alan

some more upbeat news about the example error , it's gone :-)

It's the first time I do this commit patch procedure , so I just did

git format-patch -1

with my new changes (after doing a new branch, from the master, then 
patching your changes, wrote my changes,
commited, merged into master, did a new patch, phew :-) )

I did not wrote much in the patch , but here it is, basically I reintroduced 
the wxPLplotwindow::OnCreate() event (keeping the wxPLplotwindow::Create() 
also)

I think what was happening is that the PLViewer needs the 
wxPLplotwindow::OnCreate()  to create the stream , since
wxPLplotwindow::Create() is not called for PLViewer.


I also removed this "PLDLLIMPEXP_WX"

class PLDLLIMPEXP_WX wxPLplotwindow : public wxFrame

I searched  in all the source code and could not find this symbol ?


> That produces for gcc a symbol visibility that is similar to
> visibility on Windows platforms. That is, I don't think your changes
> would have built on Windows.

It builds on Windows.
also, I think

class "something" wxPLplotwindow : public wxFrame

I believe is not a valid C++ construct

let me know if something on my patch is not clear


-Pedro


----- Original Message ----- 
From: "Alan W. Irwin" <ir...@be...>
To: "Pedro Vicente" <ped...@sp...>
Cc: "Phil Rosenberg" <p.d...@gm...>; "PLplot development list" 
<Plp...@li...>
Sent: Friday, December 23, 2016 2:50 PM
Subject: Re: [Plplot-devel] Infinite Yielding issue


> On 2016-12-23 01:40-0500 Pedro Vicente wrote:
>
>> Alan, Phil
>>
>> I went ahead and did a patch commit for option 3), attached,
>> that has this:
>>
>> (note: I wrote the code in Windows, then ftp to Linux, I got some 
>> warnings
>> about line endings, just do
>> dos2unix if any thing shows up)
>
> Hi Pedro:
>
> Your change is extremly promising because for the first time I get the
> same debug output here as you do on Ubunutu.  In other words we have
> a deterministic order for the first time!
>
> However, your commit needs some code revision.  I did some of that
> here, but there is more for you to do there to avoid the new bitmap
> errors I detected when running the wxwidgets device driver which
> launches wxPLViewer and communicates with it to get the plot displayed.
> I suspect the revision you have done for the wxPLViewer code is
> somehow interfering with that use of the wxPLViewer application.
>
> Now for the details.
>
> Your formatted patch (which by the way has no Windows line endings in
> it) applied relatively cleanly here aside from minor whitespace issues, 
> but
> did not build.
>
> That build issue was caused by incorrect symbol visibility for your 
> changes.  I found that issue on
> Linux by using the -fvisibility=hidden option, e.g.,
>
> software@raven> printenv |grep FL
> CXXFLAGS=-O3 -fvisibility=hidden -Wuninitialized
> CFLAGS=-O3 -fvisibility=hidden -Wuninitialized
> FFLAGS=-O3 -Wuninitialized -Wunused
>
> That produces for gcc a symbol visibility that is similar to
> visibility on Windows platforms. That is, I don't think your changes
> would have built on Windows.
>
> The visibility fix was to replace
>
> class wxPLplotwindow : public wxFrame
>
> by
>
> class PLDLLIMPEXP_WX wxPLplotwindow : public wxFrame
>
> in bindings/wxwidgets/wxPLplotwindow.h.
>
> I attach the revised version of your patch with this change and
> several line-ending and style changes applied.  However, to quote from
> that revised commit message (in the new Tested by: section for me)
>
> I got "the same as the Ubuntu results above which is the first time the
> two platforms have the same debugging output meaning we appear to have
> a deterministic solution!"
>
> "HOWEVER. There are run-time problems (invalid bitmap) with these
> changes for wxPLViewer if you use that in conjunction with -dev
> wxwidgets so this should not be the final version of this commit."
>
> software@raven> examples/c/x01c -dev wxwidgets
> PLplot library version: 5.11.1
> 10:54:55: Debug: nanosecs since epoch = 2210969809300109: 
> plD_init_wxwidgets(): enter
> 10:54:55: Debug: nanosecs since epoch = 2210969845142231: wxPLDevice(): 
> enter
> 10:54:55: Debug: nanosecs since epoch = 2210969845351899: wxPLDevice(): gc 
> done
> 10:54:55: Debug: nanosecs since epoch = 2210969867143754: wxPLDevice(): 
> m_interactiveTextGcdc done
> 10:54:55: Debug: nanosecs since epoch = 2210969867214649: 
> SetupMemoryMap(): enter
> 10:54:55: Debug: nanosecs since epoch = 2210969868074842: 
> SetupMemoryMap(): mapName start
> 10:54:55: Debug: nanosecs since epoch = 2210969868110859: 
> SetupMemoryMap(): mapName done
> 10:54:55: Debug: nanosecs since epoch = 2210969868140426: 
> SetupMemoryMap(): m_outputMemoryMap.create call
> 10:54:55: Debug: nanosecs since epoch = 2210969868201560: 
> SetupMemoryMap(): m_outputMemoryMap.create done
> 10:54:55: Debug: nanosecs since epoch = 2210970015888076: 
> wxPLplotwindow::wxPLplotwindow
> 10:54:55: Debug: nanosecs since epoch = 2210970031998161: 
> SetupMemoryMap(): leave
> 10:54:55: Debug: nanosecs since epoch = 2210970032100417: wxPLDevice(): 
> leave
> 10:54:55: Debug: nanosecs since epoch = 2210970032134179: 
> plD_init_wxwidgets(): leave
> software@raven> ../src/gtk/bitmap.cpp(923): assert "IsOk()" failed in 
> GetWidth(): invalid bitmap
>
> So Pedro, my version of your patch should be applied there with
>
> git am < /tmp/0001-wxwidgets-binding-fix-for-delayed-OnCreate-event.patch
>
> You should use that command on a fresh topic branch without your
> changes, i.e., latest master from SF.  And make sure it is my
> version of your patch you apply and not your own patch of the same name.
>
> Please use my revision of your patch that way to preserve the style and 
> line ending
> issues I fixed and the tested-by section I inserted in the commit message
> rather than just cherry-picking the visibility fix.
>
> The next step after that is to build the wxwidgets, wxPLViewer, and
> x01c targets and try the experiment above.  Assuming you will see the same
> bitmap error as above, then please revise your patch again to
> fix that.
>
> (Just use "git add" to stage additional changes you want to add to
> your commit, and "git commit --amend" to add those staged changes to
> your commit.  This very common git pattern avoids a series of
> non-working commits which are successive approximations to the final
> working commit.)
>
> Then follow up by testing the second revision of your patch on all
> systems accessible to you by building the test_wxPLplotDemo and
> test_c_wxwidgets targets on each of them.  (The latter does the above
> experiment on a small subset of the examples with all prerequisite
> targets built first automatically.)  Then send that second revised
> version of your patch to Phil and me for more testing/analysis.
>
> If it passes Phil's tests and mine, AND he likes your new approach,
> then I will revise my "Tested by:" section in the commit message
> appropriately and push your patch to finally put this release-critical
> issue to rest.
>
> Alan
> __________________________
> Alan W. Irwin
>
> Astronomical research affiliation with Department of Physics and 
> Astronomy,
> University of Victoria (astrowww.phys.uvic.ca).
>
> Programming affiliations with the FreeEOS equation-of-state
> implementation for stellar interiors (freeeos.sf.net); the Time
> Ephemerides project (timeephem.sf.net); PLplot scientific plotting
> software package (plplot.sf.net); the libLASi project
> (unifont.org/lasi); the Loads of Linux Links project (loll.sf.net);
> and the Linux Brochure Project (lbproject.sf.net).
> __________________________
>
> Linux-powered Science
> __________________________