On 2015-01-21 22:01-0000 Phil Rosenberg wrote:

>> I suggest you create a patch with git format-patch and send it to this
list via an attachment

Attached, any feedback, good or bad is very welcome.

Hi Phil:

Here is what I did from the git perspective (for anyone else
who wants to try your work but who is a little inexperienced
with the git am command).

1. Create private topic branch from the tip of the master
branch for evaluation of your work.

git checkout master
git checkout -b wxwidgets

2. Apply the 34 (!) commits in your present wxwidgets rewrite effort,
and check for any issues with creating those commits

git am ~irwin/Phil.Rosenberg/20150122/wxRedesign_v1.patch \
& /tmp/wxRedesign_v1.patch.out

When I looked at /tmp/wxRedesign_v1.patch.out, the only issues
concerned some trailing white space in some of your code (which
apparently my git client is configured to look for).  So at some point
as a low priority, you will want to deal with that trailing white
space issue.

In sum, "git format-patch" on your part and "git am" on my part worked like a charm, and I highly encourage this approach for sharing
changes that are not quite ready to be committed to the master branch
and pushed.

Here is what I did to evaluate your rewritten wxwidgets device code.

1. Just as an experiment I attempted to build the wxwidgets device against 
version 2.8
of the external wxwidgets software (the version I have access to on my Debian 
stable machine
unless I use the epa_build approach to build version 3.0 of the
external wxwidgets software).

make wxwidgets >& wxwidgets.out

I assumed (from what you said before about your code being dependent
on wxwidgets-3.0 features) that build would not work, but
in fact it does with no warnings or errors at all!

2. I then built C standard example 1 and attempted to run it with -dev
wxwidgets.

make x01c
examples/c/x01c -dev wxwidgets

which generated the error message
sh: 1: wxPLViewer: not found

3. That issue is a build-system dependency issue, where to solve it
you need to add the line

add_dependencies(wxwidgets wxPLViewer)

in our CMake code so that the wxPLViewer target will always be built
before the wxwidgets target.  I worked around this issue by building
wxPLViewer

make wxPLViewer >& wxPLViewer.out

Again, there were no warnings or errors with this build.

4. After that workaround

examples/c/x01c -dev wxwidgets

worked with no hangs or other obvious run-time issues like segfaults.  In fact,

valgrind examples/c/x01c -dev wxwidgets

reported no memory management issues at all except when I exited from the GUI 
(by clicking
on the file button menu and taking the only (exit) action there).
Here are the results from that valgrind run:

software@raven> valgrind examples/c/x01c -dev wxwidgets
==30922== Memcheck, a memory error detector
==30922== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==30922== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==30922== Command: examples/c/x01c -dev wxwidgets
==30922== PLplot library version: 5.10.0
==30922== Mismatched free() / delete / delete []
==30922==    at 0x4C279DC: operator delete(void*) (vg_replace_malloc.c:457)
==30922==    by 0x6E08EA1: PLMemoryMap::~PLMemoryMap() (in 
/home/software/plplot/HEAD/build_dir/drivers/wxwidgets.so)
==30922==    by 0x6E09245: wxPLDevice::~wxPLDevice() (in 
/home/software/plplot/HEAD/build_dir/drivers/wxwidgets.so)
==30922==    by 0x6E089E5: plD_tidy_wxwidgets(PLStream*) (in 
/home/software/plplot/HEAD/build_dir/drivers/wxwidgets.so)
==30922==    by 0x4E5077E: c_plend1 (in 
/home/software/plplot/HEAD/build_dir/src/libplplot.so.12.0.1)
==30922==    by 0x4E507E2: c_plend (in 
/home/software/plplot/HEAD/build_dir/src/libplplot.so.12.0.1)
==30922==    by 0x4014EB: main (in 
/home/software/plplot/HEAD/build_dir/examples/c/x01c)
==30922==  Address 0x6ab1380 is 0 bytes inside a block of size 16 alloc'd
==30922==    at 0x4C28147: operator new[](unsigned long) 
(vg_replace_malloc.c:348)
==30922==    by 0x6E08D21: PLMemoryMap::create(char const*, int, bool) (in 
/home/software/plplot/HEAD/build_dir/drivers/wxwidgets.so)
==30922==    by 0x6E0AD30: wxPLDevice::EndPage(PLStream*) (in 
/home/software/plplot/HEAD/build_dir/drivers/wxwidgets.so)
==30922==    by 0x4E4DC26: plP_eop (in 
/home/software/plplot/HEAD/build_dir/src/libplplot.so.12.0.1)
==30922==    by 0x4E5047B: c_plend1 (in 
/home/software/plplot/HEAD/build_dir/src/libplplot.so.12.0.1)
==30922==    by 0x4E507E2: c_plend (in 
/home/software/plplot/HEAD/build_dir/src/libplplot.so.12.0.1)
==30922==    by 0x4014EB: main (in 
/home/software/plplot/HEAD/build_dir/examples/c/x01c)
==30922== ==30922== ==30922== HEAP SUMMARY:
==30922==     in use at exit: 90,589 bytes in 255 blocks
==30922==   total heap usage: 47,456 allocs, 47,201 frees, 34,527,902 bytes 
allocated
==30922== ==30922== LEAK SUMMARY:
==30922==    definitely lost: 88 bytes in 1 blocks
==30922==    indirectly lost: 68 bytes in 2 blocks
==30922==      possibly lost: 0 bytes in 0 blocks
==30922==    still reachable: 90,433 bytes in 252 blocks
==30922==         suppressed: 0 bytes in 0 blocks
==30922== Rerun with --leak-check=full to see details of leaked memory
==30922== ==30922== For counts of detected and suppressed errors, rerun with: -v
==30922== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 6 from 6)

Presumably some attention to the exit code for the wxwidgets device
driver will fix this error.

So it appears you have made a promising start on the rewrite and
simplification of the wxwidgets device.

However, the rewritten wxwidgets device still has at least two major
rendering issues which are

(1) The plotted results are seriously incomplete (see the attached
screenshot of x01 results where many of the lines are missing)

(2) The 3D geometric projections of at least 3D plot labelling
need work. (See the attached screenshot of standard example 8 where not only the
surface plots are missing (see issue 1) but also the 3D labels
have an incorrect affine transformation applied.)

Since I don't get memory management issues until exit, and I certainly
don't get hangs, you might want to try version 2.8 of wxwidgets
yourself on Ubuntu to replicate the above rendering issues and fix
them.  Or if you think the above rendering issues are due to my use of
wxwidgets 2.8, then I am willing to try further experiments with
epa_built wxwidgets-3.0 to see if I can replicate your hangs.  Of
course, in any case I would strongly advise use of valgrind (as above,
but using

CXXFLAGS=-g
CFLAGS=-g

before a fresh invocation of cmake so that valgrind lists the relevant
source code lines for each error) to see whether there are
any relevant memory management issues that are causing the hangs
for you when you use version 3.0.0 of wxwidgets on Ubuntu.

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
__________________________
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Plplot-devel mailing list
Plplot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/plplot-devel

Reply via email to