Menu
▾
▴
plplot-devel
|
From: Alan W. I. <ir...@be...> - 2017-09-29 22:38:31
|
Hi Phil: Just to remind you, all device drivers are supposed to use the even-odd fill rule if pls->dev_eofill is 1 and the non-zero fill rule otherwise where the differences between the two fill rules are nicely illustrated in <https://en.wikipedia.org/wiki/Nonzero-rule>. And if you look at the wxPLDevice::FillPolygon code in drivers/wxwidgets_dev.cpp, all appears to be well, i.e., we have the following logic: if ( pls->dev_eofill ) { m_dc->DrawPolygon( pls->dev_npts, points, xoffset, yoffset, wxODDEVEN_RULE ); } else { m_dc->DrawPolygon( pls->dev_npts, points, xoffset, yoffset, wxWINDING_RULE ); } Furthermore, pls->dev_eofill is by default 0, but it can be set to 1 using the -eofill command-line option. And standard example 27 has lots of nice pages where plfill is called for complex self-intersecting paths. So running that example using, e.g., examples/c/x27c -dev <device> and examples/c/x27c -dev <device> -eofill illustrates the large differences between the two different fill rules when the device is one of our interactive devices other than wxwidgets (e.g., xwin, tk, xwin, xcairo, or qtwidget), but -dev wxwidgets has a bug where the wxPLViewer app it communicates with only shows the non-zero fill rule result regardless of whether -eofill is used or not. My recent attempt to debug that issue with gdb shows that wxPLDevice::FillPolygon does nothing other than to immediately return because m_dc is NULL. So although I was able to confirm with gdb that pls->dev_eofill is set properly by the -eofill command-line option, the routine never gets to the above m_dc->DrawPolygon calls and some other wxwidgets-related PLplot logic appears to be responding to the plfill calls in example 27 using a fixed non-zero fill rule, i.e., wxPLViewer rendered fill results use the non-zero fill rule for that example. <aside> I am pretty sure from these results that the non-zero fill rule is likely the default provided by the wxWidgets libraries since that is the default X, Qt, and pango/cairo library fill rule, and that (fixed) fill rule is what we are currently getting out of -dev wxwidgets (and wxPLViewer). Of course, that conclusion is completely contradicted by <http://docs.wxwidgets.org/3.0/classwx_d_c.html> which states the odd-even fill rule is the default for the wxWidgets libraries, but I am fairly sure that documentation is just flat-out wrong in this particular regard. </aside> So given that background, here are two questions for you which I hope will be easy for you to answer. 1. Why is m_dc always NULL for repeat calls to plfill effectively making wxPLDevice::FillPolygon a no-op? 2. What alternative routine in our wxwidgets-related code actually responds to calls to plfill? Once you give me the answer to 2. I think there is a good chance it will be a simple matter for me to install pls->dev_eofill testing logic similar to the above to specify the fill rule we want to be used at run-time as opposed to the fixed non-zero fill rule that is now being used. 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 __________________________ |
|
From: Alan W. I. <ir...@be...> - 2017-09-30 06:31:45
|
On 2017-09-30 02:27+0100 p.d...@gm... wrote: > I suspect this will be a bug in the render buffer, where the fill method is not correctly stored in or read from the buffer. You can check this by doing a plreplot with and device and seeing if the correct behaviour is maintained. To Phil and Jim: @Jim: I am specifically also addressing you in this discussion because of your knowledge of the plbuf code so I hope you will be able to reply to the final question below. @Phil: Thanks for your explanation of why rendering operations would be no-ops when using gdb on an example such as examples/c/x27c for the -dev wxwidgets case. Also, I have some contradictory evidence regarding what you suspect above. For example when I search src/plbuf.c for "eofill" there is nothing. Also, when I search that code for anything to do with fills, the fill state appears to only include information concerning discrete fills (with line patterns as in example 15) rather than information like pls->dev_eofill that is needed for solid fills. So I suspect adding that information to the fill state might solve this issue. However, without attempting to make such a change (yet) if I run examples/c/x27c -dev qtwidget -eofill or examples/c/x27c -dev xwin -eofill I get the correct behaviour (an odd-even rule fill) regardless of whether I resize the GUI or call plreplot after each call to spiro in examples/c/x27c.c. Aren't both of those actions supposed to use the plbuf code to replot the buffer? And from the above search of the src/plbuf.c code how is it possible that pls->dev_eofill is used properly by this code when no reference to that exists within that code. @Phil and Jim: I obviously must be missing something about the plbuf code, but what? 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 __________________________ |
|
From: Alan W. I. <ir...@be...> - 2017-09-30 18:35:38
|
On 2017-09-30 15:06+0100 Phil Rosenberg wrote: > Hi Alan > > > On 30 September 2017 at 07:31, Alan W. Irwin <ir...@be...> wrote: >> On 2017-09-30 02:27+0100 p.d...@gm... wrote: >> >>> I suspect this will be a bug in the render buffer, where the fill >> >> method is not correctly stored in or read from the buffer. You can >> check this by doing a plreplot with and device and seeing if the >> correct behaviour is maintained. >> >> To Phil and Jim: >> >> @Jim: >> >> I am specifically also addressing you in this discussion because of >> your knowledge of the plbuf code so I hope you will be able to reply >> to the final question below. >> >> >> Also, I have some contradictory evidence regarding what you suspect >> above. For example when I search src/plbuf.c for "eofill" there is >> nothing. Also, when I search that code for anything to do with fills, >> the fill state appears to only include information concerning discrete >> fills (with line patterns as in example 15) rather than information >> like pls->dev_eofill that is needed for solid fills. So I suspect >> adding that information to the fill state might solve this issue. >> >> However, without attempting to make such a change (yet) if I run >> >> examples/c/x27c -dev qtwidget -eofill >> >> or >> >> examples/c/x27c -dev xwin -eofill >> >> I get the correct behaviour (an odd-even rule fill) regardless of >> whether I resize the GUI or call plreplot after each call to >> spiro in examples/c/x27c.c. Aren't both of those actions supposed >> to use the plbuf code to replot the buffer? And from the above >> search of the src/plbuf.c code how is it possible that pls->dev_eofill is >> used properly by this code when no >> reference to that exists within that code. >> >> @Phil and Jim: >> >> I obviously must be missing something about the plbuf code, but what? >> > > > Having just had a quick look at the code, args.c has no mention of the > buffer at all. I suspect that what is happening with your test case is > that even when the buffer is parsed the same plStream struct is used > with the flag correctly set. The buffer will never overwrite this flag > because it never recorded it in the first place. Hi Phil: Your reply inadvertently dropped Jim and the list from the addressees so I have reinstated those for obvious reasons. I have also changed the subject line to the current topics that have been raised during this discussion. Your above remark does appear to give a rational explanation of why the above experiments worked. Therefore, there must be something else special to the -dev wxwidgets case that makes that case not work. By the way, other command-line options such as examples/c/x27c -dev wxwidgets -geometry 300x200 do work fine for -dev wxwidgets so the issue with the -eofill option with -dev wxwidgets is likely not a general command-line option issue with -dev wxwidgets but something specific about how the -eofill option is handled for the -dev wxwidgets case. > > Something that I often wondered with regards to these options - why > are they not dealt with via plP_state? These are things that change > the state of the stream and then all the buffer recording would be > performed in one location. Also I tend to feel that a function > plsfillrule( PLINT rule ) would be much more intuitive than messing > around with strings and calling plparseopts(). Similarly with other > options like setting the driver etc. > > So, I think the best way to fix this is to have the various args.c > routines call plP_state() to pass the information in question to the > drivers and get it recorded to the buffer. If I remember correctly, Andrew Ross tried (way back when) to implement what he thought were some important yet simplifying changes to the src/plargs.c code, but he could not get them to work so had to reluctantly abandon that development effort despite his huge familiarity with the PLplot code at that time. So that is simultaneously a warning about the complexity of the plargs.c implemention and a huge motivation to reimplement it in a way that is much easier to understand. And if it turns out the -eofill issue with wxwidgets is because of some subtle issue with src/plargs.c that is another big motivation to reimplement that code. Also, if you are going to tackle this project you (and Jim) should probably also look hard at Maurice's historical suggestion concerning streams which was as follows: ___________________________ Date: Tue, 10 Dec 2002 02:52:43 -0600 (CST) From: Maurice LeBrun <mj...@ga...> To: Alan W. Irwin <ir...@be...> Cc: PLplot development list <Plp...@li...> Subject: Re: [Plplot-devel] plinit, plend, plinit sequence now works, but I am having second thoughts I can't tell you the specific answer to your questions due to how maniacally busy I am these days (having just joined Lightspeed Semiconductor), but I can elucidate some of the plplot design ideas that have historically been un-documented. And, I can give it in an object-oriented context, which (because it is "canonical") is a lot nicer than the "this is the way it should work" approach I've used historically. :) This also includes proposals for change to how we do it now -- i.e. the behavior of plinit(). I've always been somewhat bothered by the way stream 0 vs stream N is handled (this bugged me back in '94 but I wasn't exactly swimming in free time then either). When plplot starts, you have the statically pre-allocated stream, stream 0. Yes the stream 0 that I hate b/c it's not allocated on the heap like a proper data-structure/object (in the plframe widget I automatically start from stream 1). So stream 0 is like a class definition and an instance rolled into one. What I think we need to do is get rid of the "instance" part of this and leave stream 0 as a "class definition" only. In this case all the command line arguments and initial pls...() calls (before plinit) serve to override the default initializion of the class variables, i.e. set stream 0 parameters only In other words, stream 0 becomes the template for all plplot streams. You can use it, but once you've called plinit() you have your own "instance" of the plplot "object" -- i.e. you have a new stream with all the relevant state info copied from stream 0. If you change it, it dies when your stream dies with plend1(). If you really want to change stream 0 (i.e. the "plplot object" "class data") you can always set your stream number to 0 and fire away. To summarize: plinit() creates a new stream, copied from stream 0 plend1() deletes that stream plend() deletes all streams, except of course stream 0 which is "class data" ___________________________ Maurice's idea was never implemented, but if this idea excites you and you would feel it would simplify rewriting src/plargs.c, then you (or Jim) should do the streams change first, and if that passes all comprehensive tests (which I would be happy to help with), then do the rewrite of src/plargs.c afterwards. @Phil and Jim: Anyhow, I strongly encourage you to try anything you like here with streams and src/plargs.c with the obvious caveats that the resulting code should be easier to understand and give as good or better results than the current code when comprehensive tests are applied. It also "would be nice" if these changes solved the original problem that started this discussion which is to get the -eofill option to work properly for -dev wxwidgets, but we don't have the knowledge at this stage to decide whether that fix is orthogonal or not to the discussed changes in streams and src/plargs.c so "time will tell" on that issue. 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 __________________________ |
|
From: Phil R. <p.d...@gm...> - 2017-10-01 00:11:15
|
Hi Alan On 30 September 2017 at 19:35, Alan W. Irwin <ir...@be...> wrote: > On 2017-09-30 15:06+0100 Phil Rosenberg wrote: > >> Hi Alan >> >> >> On 30 September 2017 at 07:31, Alan W. Irwin <ir...@be...> >> wrote: >>> >>> On 2017-09-30 02:27+0100 p.d...@gm... wrote: >>> >>>> I suspect this will be a bug in the render buffer, where the fill >>> >>> >>> method is not correctly stored in or read from the buffer. You can >>> check this by doing a plreplot with and device and seeing if the >>> correct behaviour is maintained. >>> >>> To Phil and Jim: >>> >>> @Jim: >>> >>> I am specifically also addressing you in this discussion because of >>> your knowledge of the plbuf code so I hope you will be able to reply >>> to the final question below. >>> >>> >>> Also, I have some contradictory evidence regarding what you suspect >>> above. For example when I search src/plbuf.c for "eofill" there is >>> nothing. Also, when I search that code for anything to do with fills, >>> the fill state appears to only include information concerning discrete >>> fills (with line patterns as in example 15) rather than information >>> like pls->dev_eofill that is needed for solid fills. So I suspect >>> adding that information to the fill state might solve this issue. >>> >>> However, without attempting to make such a change (yet) if I run >>> >>> examples/c/x27c -dev qtwidget -eofill >>> >>> or >>> >>> examples/c/x27c -dev xwin -eofill >>> >>> I get the correct behaviour (an odd-even rule fill) regardless of >>> whether I resize the GUI or call plreplot after each call to >>> spiro in examples/c/x27c.c. Aren't both of those actions supposed >>> to use the plbuf code to replot the buffer? And from the above >>> search of the src/plbuf.c code how is it possible that pls->dev_eofill is >>> used properly by this code when no >>> reference to that exists within that code. >>> >>> @Phil and Jim: >>> >>> I obviously must be missing something about the plbuf code, but what? >>> >> >> >> Having just had a quick look at the code, args.c has no mention of the >> buffer at all. I suspect that what is happening with your test case is >> that even when the buffer is parsed the same plStream struct is used >> with the flag correctly set. The buffer will never overwrite this flag >> because it never recorded it in the first place. > > > Hi Phil: > > Your reply inadvertently dropped Jim and the list from the addressees so > I have reinstated those for obvious reasons. I have also changed the > subject line to the current topics that have been raised during this > discussion. Sorry, must have forgot to hit reply all > > Your above remark does appear to give a rational explanation of why > the above experiments worked. Therefore, there must be something else > special to the -dev wxwidgets case that makes that case not work. By > the way, other command-line options such as > > examples/c/x27c -dev wxwidgets -geometry 300x200 > > do work fine for -dev wxwidgets so the issue with the -eofill option > with -dev wxwidgets is likely not a general command-line option issue > with -dev wxwidgets but something specific about how the -eofill > option is handled for the -dev wxwidgets case. Does geometry set the size of the canvas? This is specifically transmitted to the viewer as it is not recorded in the buffer anywhere. But is you check plbuf_bop you can see the state parameters that are recorded at the beginning of each page. I have now added the dev_eofill variable to that list. I have also set the function opt_eofill to call plP_State so that if a change occurs later in the execution it will get picked up. Should you feel the need to write a plsfillrule() function then all the stuff is there so that if you just duplicated opt_eofill then all the code is there so that the buffer would catch it. Note that I have tested this up to the point of checking in the debugger that the correct fill rule is recognised by the viewer and the correct parameter is passed in to the wxWidgets fill function. This definitely works correctly both with and without -eofill specified for example 27. However I didn't spot a difference in the output. Maybe you can check it if you know how it should look > >> >> Something that I often wondered with regards to these options - why >> are they not dealt with via plP_state? These are things that change >> the state of the stream and then all the buffer recording would be >> performed in one location. Also I tend to feel that a function >> plsfillrule( PLINT rule ) would be much more intuitive than messing >> around with strings and calling plparseopts(). Similarly with other >> options like setting the driver etc. >> >> So, I think the best way to fix this is to have the various args.c >> routines call plP_state() to pass the information in question to the >> drivers and get it recorded to the buffer. > > > If I remember correctly, Andrew Ross tried (way back when) to > implement what he thought were some important yet simplifying changes > to the src/plargs.c code, but he could not get them to work so had to > reluctantly abandon that development effort despite his huge > familiarity with the PLplot code at that time. So that is > simultaneously a warning about the complexity of the plargs.c > implemention and a huge motivation to reimplement it in a way that is > much easier to understand. And if it turns out the -eofill issue with > wxwidgets is because of some subtle issue with src/plargs.c that is > another big motivation to reimplement that code. > > Also, if you are going to tackle this project you (and Jim) should > probably also look hard at Maurice's historical suggestion concerning > streams which was as follows: > > ___________________________ > Date: Tue, 10 Dec 2002 02:52:43 -0600 (CST) > From: Maurice LeBrun <mj...@ga...> > To: Alan W. Irwin <ir...@be...> > Cc: PLplot development list <Plp...@li...> > Subject: Re: [Plplot-devel] plinit, plend, plinit sequence now works, but I > am having second thoughts > > I can't tell you the specific answer to your questions due to how maniacally > busy I am these days (having just joined Lightspeed Semiconductor), but I > can > elucidate some of the plplot design ideas that have historically been > un-documented. And, I can give it in an object-oriented context, which > (because it is "canonical") is a lot nicer than the "this is the way it > should > work" approach I've used historically. :) > > This also includes proposals for change to how we do it now -- i.e. the > behavior of plinit(). I've always been somewhat bothered by the way stream > 0 > vs stream N is handled (this bugged me back in '94 but I wasn't exactly > swimming in free time then either). > > When plplot starts, you have the statically pre-allocated stream, stream 0. > Yes the stream 0 that I hate b/c it's not allocated on the heap like a > proper > data-structure/object (in the plframe widget I automatically start from > stream > 1). > > So stream 0 is like a class definition and an instance rolled into one. > What > I think we need to do is get rid of the "instance" part of this and leave > stream 0 as a "class definition" only. In this case all the command line > arguments and initial pls...() calls (before plinit) serve to override the > default initializion of the class variables, i.e. set stream 0 parameters > only > > In other words, stream 0 becomes the template for all plplot streams. You > can > use it, but once you've called plinit() you have your own "instance" of the > plplot "object" -- i.e. you have a new stream with all the relevant state > info > copied from stream 0. If you change it, it dies when your stream dies with > plend1(). > If you really want to change stream 0 (i.e. the "plplot object" "class > data") > you can always set your stream number to 0 and fire away. > > To summarize: > > plinit() creates a new stream, copied from stream 0 > plend1() deletes that stream > plend() deletes all streams, except of course stream 0 which is "class data" > > ___________________________ > > Maurice's idea was never implemented, but if this idea excites you and > you would feel it would simplify rewriting src/plargs.c, then you (or > Jim) should do the streams change first, and if that passes all > comprehensive tests (which I would be happy to help with), then do the > rewrite of src/plargs.c afterwards. > > @Phil and Jim: > > Anyhow, I strongly encourage you to try anything you like here with > streams and src/plargs.c with the obvious caveats that the resulting > code should be easier to understand and give as good or better results > than the current code when comprehensive tests are applied. It also > "would be nice" if these changes solved the original problem that > started this discussion which is to get the -eofill option to work > properly for -dev wxwidgets, but we don't have the knowledge at this > stage to decide whether that fix is orthogonal or not to the discussed > changes in streams and src/plargs.c so "time will tell" on that issue. Hmm, I do think the initialisation is a bit messy, but I am unlikely to find the time soon to look at this. I think the best strategy long term would be to pass the PLStream out to the user as an opaque void *, then have them pass it back in for each API call. This would remove the global plsc variable which I think is important long term if we ever want to be intrastream thread safe but this would be a huge API change. This is the way libcurl works by the way. |
|
From: Alan W. I. <ir...@be...> - 2017-10-01 02:22:24
|
On 2017-10-01 01:11+0100 Phil Rosenberg wrote: > On 30 September 2017 at 19:35, Alan W. Irwin <ir...@be...> wrote: >> [...] By the way, other command-line options such as >> >> examples/c/x27c -dev wxwidgets -geometry 300x200 >> >> do work fine for -dev wxwidgets so the issue with the -eofill option >> with -dev wxwidgets is likely not a general command-line option issue >> with -dev wxwidgets but something specific about how the -eofill >> option is handled for the -dev wxwidgets case. > > Does geometry set the size of the canvas? Yes. > This is specifically transmitted to > the viewer as it is not recorded in the buffer anywhere. But is you > check plbuf_bop you can see the state parameters that are recorded at > the beginning of each page. I have now added the dev_eofill variable > to that list. I have also set the function opt_eofill to call > plP_State so that if a change occurs later in the execution it will > get picked up. Should you feel the need to write a plsfillrule() > function then all the stuff is there so that if you just duplicated > opt_eofill then all the code is there so that the buffer would catch > it. > Note that I have tested this up to the point of checking in the > debugger that the correct fill rule is recognised by the viewer and > the correct parameter is passed in to the wxWidgets fill function. > This definitely works correctly both with and without -eofill > specified for example 27. However I didn't spot a difference in the > output. Maybe you can check it if you know how it should look Hi Phil: I suspect you didn't go far enough into the pages of the example to encounter the fill results for the more complex figures. If you do that, e.g., page 19 of the example (see <http://www.plplot.org/examples-data/demo27/x27.19.png> which was generated with -dev pngcairo and the -eofill option), the result should have many holes in the fill region relative to the same page generated without the -eofill option. If you don't see that difference on Windows, I feel that is likely due to a wxWidgets library bug on Windows since that difference does show up in the Linux case. Anyhow, thanks very much for this fix, and I have changed the status of https://sourceforge.net/p/plplot/bugs/174/ to closed-fixed. With regard to your remark concerning writing a plsfillrule() function and systematically using it throughout src/plargs.c, I wouldn't want to do that myself, but if you or Jim want to make such a change and it passes comprehensive testing, I certainly would not object. 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 __________________________ |
|
From: Phil R. <p.d...@gm...> - 2017-10-01 08:49:41
|
>> Note that I have tested this up to the point of checking in the >> debugger that the correct fill rule is recognised by the viewer and >> the correct parameter is passed in to the wxWidgets fill function. >> This definitely works correctly both with and without -eofill >> specified for example 27. However I didn't spot a difference in the >> output. Maybe you can check it if you know how it should look > > > Hi Phil: > > I suspect you didn't go far enough into the pages of the example to > encounter the fill results for the more complex figures. If you do > that, e.g., page 19 of the example (see > <http://www.plplot.org/examples-data/demo27/x27.19.png> which was > generated with -dev pngcairo and the -eofill option), the result > should have many holes in the fill region relative to the same page > generated without the -eofill option. If you don't see that > difference on Windows, I feel that is likely due to a wxWidgets > library bug on Windows since that difference does show up in the Linux > case. > > Anyhow, thanks very much for this fix, and I have changed the status > of https://sourceforge.net/p/plplot/bugs/174/ to closed-fixed. I get output just like that image without the -eofill parameter. I suspect that as you said this is a bug in wxWidgets on Windows. I'm not exactly sure how the differences are supposed to manifest. If you could send me a relatively simple polygon that should give different output depending upon which rule is used and a description of what should be different then I will generate a test case and submit a bug report. > > With regard to your remark concerning writing a plsfillrule() function > and systematically using it throughout src/plargs.c, I wouldn't want > to do that myself, but if you or Jim want to make such a change and it > passes comprehensive testing, I certainly would not object. I can add a new API function if you think it is useful, but I can only propagate it as far as the C and C++ APIs, someone else would have to propagate it to other languages as needed. |
|
From: Alan W. I. <ir...@be...> - 2017-10-01 19:50:55
|
On 2017-09-30 19:22-0700 Alan W. Irwin wrote: > Anyhow, thanks very much for this fix, and I have changed the status > of https://sourceforge.net/p/plplot/bugs/174/ to closed-fixed. I have just discovered a really strange problem with your recent -eofill fix (commit b603fd22). The issue is that valgrind results on C and Fortran standard examples show no memory-management issues, and you would expect that good result would continue with all bindings since your commit made changes only to C language files associated with our core C library and not the bindings. However, for some strange reason your change does cause segfaults for all C++ examples and all devices that I have tried for those examples whenever the -eofill option is used. There are no such issues when the -eofill option is not used. Here are typical valgrind results for the two cases where I have chosen to use one of our simpler C++ examples (x00) and one of our simpler devices (svg). software@raven> valgrind examples/c++/x00 -dev svg -o test.svg ==1930== Memcheck, a memory error detector ==1930== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==1930== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info ==1930== Command: examples/c++/x00 -dev svg -o test.svg ==1930== ==1930== ==1930== HEAP SUMMARY: ==1930== in use at exit: 0 bytes in 0 blocks ==1930== total heap usage: 463 allocs, 463 frees, 148,933 bytes allocated ==1930== ==1930== All heap blocks were freed -- no leaks are possible ==1930== ==1930== For counts of detected and suppressed errors, rerun with: -v ==1930== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) software@raven> valgrind examples/c++/x00 -dev svg -o test.svg -eofill ==1931== Memcheck, a memory error detector ==1931== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==1931== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info ==1931== Command: examples/c++/x00 -dev svg -o test.svg -eofill ==1931== ==1931== Invalid read of size 8 ==1931== at 0x5C3656E: plP_state (plcore.c:257) ==1931== by 0x5C24C42: opt_eofill (plargs.c:2845) ==1931== by 0x5C22BED: ProcessOpt (plargs.c:1140) ==1931== by 0x5C22A1D: ParseOpt (plargs.c:1068) ==1931== by 0x5C22779: c_plparseopts (plargs.c:935) ==1931== by 0x4E40719: plstream::parseopts(int*, char**, int) (plstream.cc:1283) ==1931== by 0x400C4C: x00::x00(int, char**) (x00.cc:59) ==1931== by 0x400D92: main (x00.cc:79) ==1931== Address 0x48 is not stack'd, malloc'd or (recently) free'd ==1931== ==1931== ==1931== Process terminating with default action of signal 11 (SIGSEGV) ==1931== Access not within mapped region at address 0x48 ==1931== at 0x5C3656E: plP_state (plcore.c:257) ==1931== by 0x5C24C42: opt_eofill (plargs.c:2845) ==1931== by 0x5C22BED: ProcessOpt (plargs.c:1140) ==1931== by 0x5C22A1D: ParseOpt (plargs.c:1068) ==1931== by 0x5C22779: c_plparseopts (plargs.c:935) ==1931== by 0x4E40719: plstream::parseopts(int*, char**, int) (plstream.cc:1283) ==1931== by 0x400C4C: x00::x00(int, char**) (x00.cc:59) ==1931== by 0x400D92: main (x00.cc:79) ==1931== If you believe this happened as a result of a stack ==1931== overflow in your program's main thread (unlikely but ==1931== possible), you can try to increase the size of the ==1931== main thread stack using the --main-stacksize= flag. ==1931== The main thread stack size used in this run was 8388608. ==1931== ==1931== HEAP SUMMARY: ==1931== in use at exit: 29,775 bytes in 265 blocks ==1931== total heap usage: 314 allocs, 49 frees, 74,436 bytes allocated ==1931== ==1931== LEAK SUMMARY: ==1931== definitely lost: 0 bytes in 0 blocks ==1931== indirectly lost: 0 bytes in 0 blocks ==1931== possibly lost: 0 bytes in 0 blocks ==1931== still reachable: 29,775 bytes in 265 blocks ==1931== suppressed: 0 bytes in 0 blocks ==1931== Rerun with --leak-check=full to see details of leaked memory ==1931== ==1931== For counts of detected and suppressed errors, rerun with: -v ==1931== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) Segmentation fault 1930 (without -eofill) shows perfect valgrind results while 1931 shows shows showstopping (segfault) memory management issues with -eofill. I hope you can figure out this peculiar issue with your fix because it has me completely stumped! 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 __________________________ |
|
From: Phil R. <p.d...@gm...> - 2017-10-01 23:11:25
|
Fixed My test for initialisation was incorrect. On 1 October 2017 at 20:50, Alan W. Irwin <ir...@be...> wrote: > On 2017-09-30 19:22-0700 Alan W. Irwin wrote: > >> Anyhow, thanks very much for this fix, and I have changed the status >> of https://sourceforge.net/p/plplot/bugs/174/ to closed-fixed. > > > I have just discovered a really strange problem with your recent > -eofill fix (commit b603fd22). The issue is that valgrind results on C > and Fortran standard examples show no memory-management issues, and > you would expect that good result would continue with all bindings > since your commit made changes only to C language files associated > with our core C library and not the bindings. However, for some > strange reason your change does cause segfaults for all C++ examples > and all devices that I have tried for those examples whenever the > -eofill option is used. There are no such issues when the -eofill > option is not used. > > Here are typical valgrind results for the two cases where I > have chosen to use one of our simpler C++ examples (x00) > and one of our simpler devices (svg). > > software@raven> valgrind examples/c++/x00 -dev svg -o test.svg > ==1930== Memcheck, a memory error detector > ==1930== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. > ==1930== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info > ==1930== Command: examples/c++/x00 -dev svg -o test.svg > ==1930== ==1930== ==1930== HEAP SUMMARY: > ==1930== in use at exit: 0 bytes in 0 blocks > ==1930== total heap usage: 463 allocs, 463 frees, 148,933 bytes allocated > ==1930== ==1930== All heap blocks were freed -- no leaks are possible > ==1930== ==1930== For counts of detected and suppressed errors, rerun with: > -v > ==1930== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) > software@raven> valgrind examples/c++/x00 -dev svg -o test.svg -eofill > ==1931== Memcheck, a memory error detector > ==1931== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. > ==1931== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info > ==1931== Command: examples/c++/x00 -dev svg -o test.svg -eofill > ==1931== ==1931== Invalid read of size 8 > ==1931== at 0x5C3656E: plP_state (plcore.c:257) > ==1931== by 0x5C24C42: opt_eofill (plargs.c:2845) > ==1931== by 0x5C22BED: ProcessOpt (plargs.c:1140) > ==1931== by 0x5C22A1D: ParseOpt (plargs.c:1068) > ==1931== by 0x5C22779: c_plparseopts (plargs.c:935) > ==1931== by 0x4E40719: plstream::parseopts(int*, char**, int) > (plstream.cc:1283) > ==1931== by 0x400C4C: x00::x00(int, char**) (x00.cc:59) > ==1931== by 0x400D92: main (x00.cc:79) > ==1931== Address 0x48 is not stack'd, malloc'd or (recently) free'd > ==1931== ==1931== ==1931== Process terminating with default action of signal > 11 (SIGSEGV) > ==1931== Access not within mapped region at address 0x48 > ==1931== at 0x5C3656E: plP_state (plcore.c:257) > ==1931== by 0x5C24C42: opt_eofill (plargs.c:2845) > ==1931== by 0x5C22BED: ProcessOpt (plargs.c:1140) > ==1931== by 0x5C22A1D: ParseOpt (plargs.c:1068) > ==1931== by 0x5C22779: c_plparseopts (plargs.c:935) > ==1931== by 0x4E40719: plstream::parseopts(int*, char**, int) > (plstream.cc:1283) > ==1931== by 0x400C4C: x00::x00(int, char**) (x00.cc:59) > ==1931== by 0x400D92: main (x00.cc:79) > ==1931== If you believe this happened as a result of a stack > ==1931== overflow in your program's main thread (unlikely but > ==1931== possible), you can try to increase the size of the > ==1931== main thread stack using the --main-stacksize= flag. > ==1931== The main thread stack size used in this run was 8388608. > ==1931== ==1931== HEAP SUMMARY: > ==1931== in use at exit: 29,775 bytes in 265 blocks > ==1931== total heap usage: 314 allocs, 49 frees, 74,436 bytes allocated > ==1931== ==1931== LEAK SUMMARY: > ==1931== definitely lost: 0 bytes in 0 blocks > ==1931== indirectly lost: 0 bytes in 0 blocks > ==1931== possibly lost: 0 bytes in 0 blocks > ==1931== still reachable: 29,775 bytes in 265 blocks > ==1931== suppressed: 0 bytes in 0 blocks > ==1931== Rerun with --leak-check=full to see details of leaked memory > ==1931== ==1931== For counts of detected and suppressed errors, rerun with: > -v > ==1931== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) > Segmentation fault > > 1930 (without -eofill) shows perfect valgrind results while 1931 shows > shows showstopping (segfault) memory management issues with -eofill. > > I hope you can figure out this peculiar issue with your fix because > it has me completely stumped! > > > 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 > __________________________ |
|
From: Alan W. I. <ir...@be...> - 2017-10-01 20:01:23
|
On 2017-10-01 09:49+0100 Phil Rosenberg wrote: [Alan said] >> With regard to your remark concerning writing a plsfillrule() function >> and systematically using it throughout src/plargs.c, I wouldn't want >> to do that myself, but if you or Jim want to make such a change and it >> passes comprehensive testing, I certainly would not object. > [Phil responded] > I can add a new API function if you think it is useful, but I can only > propagate it as far as the C and C++ APIs, someone else would have to > propagate it to other languages as needed. > >From what has been said, my impression is a plsfillrule() function is C-only functionality to make src/plargs.c easier to understand and use correctly. If that impression is correct there should be no need to propagate this functionality even to our C++ binding since all our bindings simply wrap the C plparseopts routine without knowing its internal implementation details. But please educate me if that impression is incorrect. 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 __________________________ |
|
From: Phil R. <p.d...@gm...> - 2017-10-01 23:14:58
|
On 1 October 2017 at 21:01, Alan W. Irwin <ir...@be...> wrote: > On 2017-10-01 09:49+0100 Phil Rosenberg wrote: > > [Alan said] >>> >>> With regard to your remark concerning writing a plsfillrule() function >>> and systematically using it throughout src/plargs.c, I wouldn't want >>> to do that myself, but if you or Jim want to make such a change and it >>> passes comprehensive testing, I certainly would not object. >> >> > [Phil responded] >> >> I can add a new API function if you think it is useful, but I can only >> propagate it as far as the C and C++ APIs, someone else would have to >> propagate it to other languages as needed. >> > > From what has been said, my impression is a plsfillrule() function is > C-only functionality to make src/plargs.c easier to understand and use > correctly. If that impression is correct there should be no need to > propagate this functionality even to our C++ binding since all our bindings > simply wrap the C plparseopts routine without knowing its > internal implementation details. But please educate me if that > impression is incorrect. > Hi Alan I actually meant, do we want to add plsfillrule as an API function? It feels more like it should be an API function rather than a command argument. It would be little trouble to allow users to swap back and forward between the two rules. But I have a feeling this functionality is not used that often so maybe it's not worth the effort. Phil |
|
From: Alan W. I. <ir...@be...> - 2017-10-02 04:43:02
|
On 2017-10-02 00:14+0100 Phil Rosenberg wrote:
> On 1 October 2017 at 21:01, Alan W. Irwin <ir...@be...> wrote:
>> On 2017-10-01 09:49+0100 Phil Rosenberg wrote:
>>
>> [Alan said]
>>>>
>>>> With regard to your remark concerning writing a plsfillrule() function
>>>> and systematically using it throughout src/plargs.c, I wouldn't want
>>>> to do that myself, but if you or Jim want to make such a change and it
>>>> passes comprehensive testing, I certainly would not object.
>>>
>>>
>> [Phil responded]
>>>
>>> I can add a new API function if you think it is useful, but I can only
>>> propagate it as far as the C and C++ APIs, someone else would have to
>>> propagate it to other languages as needed.
>>>
>>
>> From what has been said, my impression is a plsfillrule() function is
>> C-only functionality to make src/plargs.c easier to understand and use
>> correctly. If that impression is correct there should be no need to
>> propagate this functionality even to our C++ binding since all our bindings
>> simply wrap the C plparseopts routine without knowing its
>> internal implementation details. But please educate me if that
>> impression is incorrect.
>>
>
> Hi Alan
> I actually meant, do we want to add plsfillrule as an API function? It
> feels more like it should be an API function rather than a command
> argument. It would be little trouble to allow users to swap back and
> forward between the two rules. But I have a feeling this functionality
> is not used that often so maybe it's not worth the effort.
Hi Phil:
Sorry, but I think I have misunderstood this subset of this thread
from the beginning since I assumed you were talking about some general
capability rather than something specific to -eofill (which is obvious
from the name of the "plsfillrule" function, but I simply missed the
significance of that name until now).
So to start over, it would be worthwhile to be able to set
pls->dev_eofill to true or false at any time. But I don't think we
need to add API to do that. Instead, we need to modify src/plargs.c
such that -eofill takes a true (non-zero) or false (0) argument.
with pls->dev_eofill being set appropriately to true or false.
Then users in any language can call
plsetopt("eofill", "1");
plsetopt("eofill", "0");
as needed. Of course, demanding that -eofill requires an argument is
backwards incompatible, but if we mention that in the release notes I
think that would be sufficient since my judgement is -eofill is not a
heavily used option.
If you agree with the above idea to add an argument to -eofill, then I
would be willing to take responsibility for implementing that and
documenting that backwards-incompatible change in README.release.
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
__________________________
|
|
From: Jim D. <ji...@di...> - 2017-10-02 05:00:54
|
> On Sep 30, 2017, at 2:35 PM, Alan W. Irwin <ir...@be...> wrote: > > On 2017-09-30 15:06+0100 Phil Rosenberg wrote: > >> Hi Alan >> >> >> On 30 September 2017 at 07:31, Alan W. Irwin <ir...@be...> wrote: >>> On 2017-09-30 02:27+0100 p.d...@gm... wrote: >>> >>>> I suspect this will be a bug in the render buffer, where the fill >>> >>> method is not correctly stored in or read from the buffer. You can >>> check this by doing a plreplot with and device and seeing if the >>> correct behaviour is maintained. >>> >>> To Phil and Jim: >>> >>> @Jim: >>> >>> I am specifically also addressing you in this discussion because of >>> your knowledge of the plbuf code so I hope you will be able to reply >>> to the final question below. >>> >>> >>> Also, I have some contradictory evidence regarding what you suspect >>> above. For example when I search src/plbuf.c for "eofill" there is >>> nothing. Also, when I search that code for anything to do with fills, >>> the fill state appears to only include information concerning discrete >>> fills (with line patterns as in example 15) rather than information >>> like pls->dev_eofill that is needed for solid fills. So I suspect >>> adding that information to the fill state might solve this issue. >>> The eofill setting is saved by the plbuf at a bop (see line 176 and 359 in plbuf.c). >>> However, without attempting to make such a change (yet) if I run >>> >>> examples/c/x27c -dev qtwidget -eofill >>> >>> or >>> >>> examples/c/x27c -dev xwin -eofill >>> >>> I get the correct behaviour (an odd-even rule fill) regardless of >>> whether I resize the GUI or call plreplot after each call to >>> spiro in examples/c/x27c.c. Aren't both of those actions supposed >>> to use the plbuf code to replot the buffer? And from the above >>> search of the src/plbuf.c code how is it possible that pls->dev_eofill is >>> used properly by this code when no >>> reference to that exists within that code. I’m confused because plbuf is saving the dev_eofill setting. >>> >>> @Phil and Jim: >>> >>> I obviously must be missing something about the plbuf code, but what? >>> >> I have noticed some inconsistencies in the mash-up of plbuf and drivers when it comes to handling state and escape (e.g. plD_esc) functions. Based on my recollection of old graphics devices, I think I understand the difference (state has to do with internal plplot and escape has to do with commands to the graphic device) but with the way graphics devices have evolved we may want to revisit the implementation (e.g. make sure no settings are getting lost on a replot). The biggest weakness in plbuf is that what it thinks should be saved may not match what is needed. When working on the core and drivers, it is easy to add a bit of state and not update plbuf. There might be a way to make plbuf be more aggressive in saving state information and restoring it when replaying the buffer. I can make the aggressiveness be a setting that a driver can set it if it wants the current behavior—I am concerned that some of the older drivers might have issues. > @Phil and Jim: > > Anyhow, I strongly encourage you to try anything you like here with > streams and src/plargs.c with the obvious caveats that the resulting > code should be easier to understand and give as good or better results > than the current code when comprehensive tests are applied. It also > "would be nice" if these changes solved the original problem that > started this discussion which is to get the -eofill option to work > properly for -dev wxwidgets, but we don't have the knowledge at this > stage to decide whether that fix is orthogonal or not to the discussed > changes in streams and src/plargs.c so "time will tell" on that issue. > Alan > __________________________ > Alan W. Irwin > |
|
From: Phil R. <p.d...@gm...> - 2017-10-02 10:00:33
|
> > The eofill setting is saved by the plbuf at a bop (see line 176 and 359 in plbuf.c). > Hi Jim - These are changes I made over the weekend. I guess you're not subscribed to get code change notifications or maybe you missed them :-) > > I have noticed some inconsistencies in the mash-up of plbuf and drivers when it comes to handling state and escape (e.g. plD_esc) functions. Based on my recollection of old graphics devices, I think I understand the difference (state has to do with internal plplot and escape has to do with commands to the graphic device) but with the way graphics devices have evolved we may want to revisit the implementation (e.g. make sure no settings are getting lost on a replot). My understanding is the same. state commands change something that plplot remembers - usually something that is saved in the PLStream, whereas escapes are commands sent to the drivers to perform an action - usually rendering. Always thought that was an odd name, when I think of escape I think of escape sequences in text etc. I guess there is a historical reason. I also guess that a number of things like setting the rotation or clipping region or subpage or other things are really changes of state, but don't go through that code. Again I guess these are for historical reasons. > > The biggest weakness in plbuf is that what it thinks should be saved may not match what is needed. When working on the core and drivers, it is easy to add a bit of state and not update plbuf. There might be a way to make plbuf be more aggressive in saving state information and restoring it when replaying the buffer. I can make the aggressiveness be a setting that a driver can set it if it wants the current behavior—I am concerned that some of the older drivers might have issues. I actually think that now that the wxWidgets interactive driver (i.e. the one that gets called from a command line application, rather than what gets called when you pass in a wxDC) uses the buffer for all its rendering, we now do rather a good job of keeping on top of these things. The biggest problems have turned out to be, things that are stored in PLStream, but don't go through the Plp_State code. The things that I think still pose challenges are 1) Initialisation and beginning the first page - it can be that when passing a buffer around we end up initialising more than once and this has caused issues. I these are mostly sorted now though. 2) Resizing - the buffer stores the coordinates for rendering and the sizes of text. But if we resize then we may want to scale the text size, or keep the text size the same and keep the distance of labels from the axis the same. Currently we keep the text size the same, but they move relative to the axes. 3) Changes of aspect ratio - This can cause a big mess, especially for rotated text on plots, axis label positioning and tick mark length. Actually now that I have listed these, I am going to add 2 and 3 to the bug tracker so they don't get totally forgotten. Phil |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X