Re: patch to be applied to default or stable ?

[Ben Abbott]

On Feb 15, 2012, at 4:45 AM, Michael Goffioul wrote:

> On Tue, Feb 14, 2012 at 10:45 PM, Ben Abbott <address@hidden> wrote:
> 
>> On Feb 13, 2012, at 9:47 AM, Ben Abbott wrote:
>> 
>>> On Feb 12, 2012, at 9:14 PM, Ben Abbott wrote:
>>> 
>>>> On Jan 28, 2012, at 11:19 PM, John W. Eaton wrote:
>>>> 
>>>>> On 28-Jan-2012, Ben Abbott wrote:
>>>>> 
>>>>> | I'm able to fix the problem. I've been reverting your reversion below 
>>>>> and thought it was a MacOS X only thing.
>>>>> |
>>>>> |   
>>>>> http://hg.savannah.gnu.org/hgweb/octave/diff/1367f2db49a2/src/graphics.cc
>>>>> |
>>>>> | With the sources up to date and reverting this specific changeset, the 
>>>>> fltk output works correctly for me.
>>>>> |
>>>>> | If you apply the simple change below, does the fltk toolkit work as 
>>>>> expected ?
>>>>> 
>>>>> Unfortunately, no.  It fails for a pair of figures that are generated
>>>>> in some horribly complicated way by an example from the MTEX package
>>>>> (http://code.google.com/p/mtex).
>>>>> 
>>>>> If you are really interested, you can try to see the problem by doing
>>>>> the following:
>>>>> 
>>>>> * Download mtex 3.2.2
>>>>> 
>>>>> * Unpack it and cd to the mtex-3.2.2 directory
>>>>> 
>>>>> * Edit the file mtex_settings.m and uncomment the line
>>>>> 
>>>>>   %set_mtex_option('GrainSelector','off')
>>>>> 
>>>>> * Create symbolic link in c/bin for your architecture.  This
>>>>> directory has some binaries (ick, I know) for systems that run
>>>>> Matlab.  For my system, I needed to do
>>>>> 
>>>>>   ln -s glnxa64 -> gnu-linux-x86_64
>>>>> 
>>>>> in the c/bin directory
>>>>> 
>>>>> * In the top-level directory, run Octave and type
>>>>> 
>>>>>   startup_mtex
>>>>> 
>>>>>   (answer no to the question about installing mtex globally)
>>>>> 
>>>>>   warning off Octave:missing-glyph
>>>>>   more off
>>>>>   graphics_toolkit fltk
>>>>>   grain_demo
>>>>> 
>>>>> You should get two plots that look something like the first pair of
>>>>> screenshots attached below.  With the current stable sources, this is
>>>>> what I see.  With your small patch applied, I get the second set of
>>>>> badly sized and blank plots.
>>>>> 
>>>>> But these plot is generated by so much code spread around in many places
>>>>> that I think it will be very hard to debug.
>>>>> 
>>>>> jwe
>>>> 
>>>> Sorry it took me so long to get back to this.
>>>> 
>>>> I've installed 3..7.0+ with tip, and *without* the 34720 patch applied.
>>>> 
>>>> hangeset:   14358:e7c74f56cd03
>>>> tag:         tip
>>>> user:        Ben Abbott  <address@hidden>
>>>> date:        Sat Feb 11 21:09:03 2012 -0500
>>>> summary:     fltk toolkit requires figure units to be "pixels". Bug # 
>>>> 35430.
>>>> 
>>>> For the symbolic link I used
>>>> 
>>>>      ln -s maci64 darwin11.3.0-x86_64
>>>> 
>>>> Unfortunately, the mex files do not build for me.
>>>> 
>>>> For example.
>>>> 
>>>> In file included from /Users/bpabbott/octave/mtex-3.2.2/c/mex/S1Grid.c:1:0,
>>>>                from 
>>>> /Users/bpabbott/octave/mtex-3.2.2/c/mex/S1Grid_find.c:19:
>>>> /Users/bpabbott/octave/mtex-3.2.2/c/mex/buffer.c:34:13: error: 
>>>> redefinition of typedef 'mwSize'
>>>> /opt/local/include/octave-3.7.0+/octave/mxarray.h:88:13: note: previous 
>>>> declaration of 'mwSize' was here
>>>> /Users/bpabbott/octave/mtex-3.2.2/c/mex/buffer.c:35:13: error: 
>>>> redefinition of typedef 'mwIndex'
>>>> /opt/local/include/octave-3.7.0+/octave/mxarray.h:89:13: note: previous 
>>>> declaration of 'mwIndex' was here
>>>> 
>>>> Did you see this ?
>>>> 
>>>> I'm not sure it is a proper fix, but I commented out the offending lines 
>>>> imn buffer.c
>>>> 
>>>> #if !defined(MWSIZE_MAX)
>>>> //typedef    int mwSize;
>>>> //typedef    int mwIndex;
>>>> //typedef    int mwSignedIndex;
>>>> #endif
>>>> 
>>>> With those lines commented out
>>>> 
>>>>      startup_mtex
>>>>      initializewarning: fopen: file found in load path
>>>>      sh: free: command not found
>>>>       MTEX 3.2.2  ... done!
>>>> 
>>>> The "free" problem is in
>>>> 
>>>>      .../mtex-3.2.2/tools/misc_tools/getmem.m
>>>> 
>>>> The line is ...
>>>> 
>>>>      [r,s] = system('free');
>>>> 
>>>> Looks like this shouldn't be a problem, but I thought I'd mention in (just 
>>>> in case).
>>>> 
>>>> The resulting figure(1) had zero width, but figure(2) looks ok. I checked 
>>>> figure(1) for units.
>>>> 
>>>>      get (1, 'units')
>>>>      ans = normalized
>>>> 
>>>> The fltk backend was only been working correctly for units == pixels. I 
>>>> pushed two changesets over the weekend to fix that.
>>>> 
>>>> I'm able to grab the edge of figure (1) and lengthen it. The result is the 
>>>> same as with your 1st pair.
>>>> 
>>>> I have to verity, but the 34720 changeset looks necessary to me. If the 
>>>> (true) is not included then neither execute_resizefcn () or 
>>>> update_boundingbox () are called. My impression is that this worked for 
>>>> you because the fltk backend wasn't updating the figure size when the 
>>>> units changed.
>>>> 
>>>> If you run this example with the same tip I used, I expect you'll now see 
>>>> a zero width figure (1) as well.
>>>> 
>>>> I'll apply the 34720 patch, reinstall, run the example again, and report 
>>>> back tomorrow.
>>>> 
>>>> Ben
>>> 
>>> I made a mistake above. The sources I installed that produced a zero width 
>>> figure (actually 1 pixel wide) had the 34720 patch applied.
>>> 
>>> Without that being applied, the figures display correctly.  The figure 
>>> units conversion looks to be functioning correctly.
>>> 
>>>       get (1, 'units')
>>>       ans = normalized
>>>       get (1, 'position')
>>>       ans =
>>> 
>>>          0.060714   0.059048   0.338095   0.212381
>>> 
>>>       set (1,'units',  'pixels')
>>>       get (1, 'position')
>>>       ans =
>>> 
>>>          103    63   568   223
>>> 
>>> It looks to me as if the 34720 patch should be applied, but that there is 
>>> something else wrong. I'll try to isolate the what the mtex demo does which 
>>> results in the 1 pixel wide plot.
>>> 
>>> Ben
>> 
>> John,
>> 
>> I've attached a changeset that fixes the problems reported in bug # 34720 
>> and the problem with running the mtex grain_demo().
>> 
>> I noticed that set_position was being called after the units property was 
>> changed, and before the position property had been updated. This resulted in 
>> some absurd position property values (when switching the units from pixels 
>> to normalized). To fix that I modified figure::properties::update_units() to 
>> work in the same manner as axes::properties::update_units().
>> 
>> I'm not sure I understand exactly what boolean inputs to position.set () are 
>> used for. There are some comments in graphics.h.in that indicate the first 
>> determines if listeners are run and the second determines if the toolkit is 
>> notified. So my changeset comments indicate that understanding. If that is 
>> wrong, I can modify the changeset's comments to something more appropriate.
>> 
>> In any event, I don't think it is a good idea to trust that fltk-aqua 
>> renders that same as fltk-x11. Does the attached changset work for you with 
>> grain_demo from mtex ?
> 
> Ben,
> 
> Could you try with the proposed patch here
> (https://mailman.cae.wisc.edu/pipermail/octave-maintainers/2011-December/025995.html)
> + your change in update_units() ?
> 
> The patch above is a more accurate way of handling things.
> 
> Michael.

Thanks Michael !

I'll do some testing (jwe's example on MacOS and Ubuntu) and push the change 
later today.

Ben