bugGNU Octave - Bugs: bug #43502, slight change to allow building...

 
 

bug #43502: slight change to allow building doc again without GraphicsMagick

Submitter:  Mike Miller <mtmiller>
Submitted:  Thu 30 Oct 2014 02:56:02 AM UTC
   
 
Category:  Configuration and Build System Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 11 Nov 2014 08:47:12 PM UTC, comment #9: 

Hi,

I have pushed the changes discussed in this thread [1].
Then the fix for this bug was as simple. I pushed it at http://hg.savannah.gnu.org/hgweb/octave/rev/346b20b2c2ff

Closing the bug report.

[1] http://octave.1599824.n4.nabble.com/Replacing-default-img-by-default-image-cdata-td4667172.html

Pantxo Diribarne <pantxo>
Group Member
Fri 31 Oct 2014 05:57:11 PM UTC, comment #8: 

Agreed for the additional comment about those strange dimensions.

Anyway I will raise the discussion on the maintainers mailing list: if people agree that 'image ()' should use the default 'cdata' instead of loading default.img, the fixme and the comments can be removed as soon as necessary modifications have been implemented.

Pantxo Diribarne <pantxo>
Group Member
Fri 31 Oct 2014 04:37:03 PM UTC, comment #7: 

Not tested yet, but the patch looks good to me. Might want to add a mention that the image dimensions must be the same as the file scripts/image/default.img, at a glance the size (53,40) looks strange.

Mike Miller <mtmiller>
Group Member
Fri 31 Oct 2014 08:10:55 AM UTC, comment #6: 

So what do you think of the attached patch?
I added a FIXME as I still think "image ()" should not load an image but rather use default cdata.

(file #32348)

Pantxo Diribarne <pantxo>
Group Member
Fri 31 Oct 2014 01:46:38 AM UTC, comment #5: 

You're right, I must have accidentally rebuilt my standard build after making this change locally. Reverted and rebuilt and I see the differences in cdata, xdata, and ydata.

Mike Miller <mtmiller>
Group Member
Thu 30 Oct 2014 06:33:32 PM UTC, comment #4: 

I just reproduced your patch and found what I expected (plus "xdata" and "ydata"):


$diff '/home/pantxo/dev/octavebuild/doc/interpreter/plot-imageproperties.texi.old' '/home/pantxo/dev/octavebuild/doc/interpreter/plot-imageproperties.texi'
61c61
< @item @code{cdata}: matrix, def. 53-by-40 uint8
---
> @item @code{cdata}: matrix, def. 2-by-2 double
175c175
< @item @code{xdata}: two elements vector, def. @code{[1   40]}
---
> @item @code{xdata}: two elements vector, def. @code{[1   2]}
183c183
< @item @code{ydata}: two elements vector, def. @code{[1   53]}
---
> @item @code{ydata}: two elements vector, def. @code{[1   2]}


Pantxo Diribarne <pantxo>
Group Member
Thu 30 Oct 2014 04:21:14 PM UTC, comment #3: 

Sorry, I have not had time to test your patch up to now, but I do expect a difference and I am a bit puzzled. We should see it in plot-imageproperties.texi, for the "cdata" property. With your patch I expect something like:

"cdata": matrix, def. 2-by-2

instead of

"cdata": matrix, def. 53-by-40

... in the current version which uses "default.img" file, or

"cdata": matrix, def. 64-by-64

... if we use "get (0, "defaultimagecdata")" as input. I'll check this out tonight.

About the warning, it's indeed the classical quantum depth ...

Pantxo Diribarne <pantxo>
Group Member
Thu 30 Oct 2014 03:03:19 PM UTC, comment #2: 

Not sure I understand. The point I was making was that using an arbitrary image didn't seem to affect the Texinfo output produced by genpropdoc.m, or at least none that I could see. I thought I diffed all of the plot*.texi files between a previous build and a new build with this change, but I may have missed something :)

If I did miss a difference, and the image that is used does result in a difference in the documentation, then I see what you're getting at, we should have a well-defined default that matches the documentation.

Agree that the warning can go away, that was for the typical quantum depth warning message from the library, right?

Mike Miller <mtmiller>
Group Member
Thu 30 Oct 2014 09:20:14 AM UTC, comment #1: 

Hi,

Thanks for taking care of this. The issue is that "image ()" loads an image from file instead of using the default image. There are two possibilities I can see:

  • The simplest: replace the "zeros (2)" by "get (0, "defaultimagecdata")" in your changeset.
  • More involved: modify image.m to let "image ()" return the default image. It is as simple as not providing a "cdata" property/value pair to _image_, but the resulting image (octave logo) is very different from the one we currently obtain, which may not be satisfactory for some demos (that I don't know of).


In any case, the "warning ("off")" and the associated comment are not needed anymore.

Pantxo Diribarne <pantxo>
Group Member
Thu 30 Oct 2014 02:56:02 AM UTC, original submission:  

This is a followup to the fix for bug #42536. The new genpropdoc.m script creates a default image object, which means it has to load the "default.img" file using GraphicsMagick. The build previously allowed one to compile and build the manual without GraphicsMagick support. This no longer works.

I've attached a trivial diff that seems to work for me, passing an empty image using zeros(), which doesn't require GraphicsMagick to be enabled in the build, and seems to produce the same output. CC'ing Pantxo who worked on adding this feature.

Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #32348:  fix_gmcall.patch added by pantxo (1KiB - application/x-download)
file #32343:  genpropdoc.diff added by mtmiller (530B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by mtmiller (Submitted the item)
  • -email is unavailable- added by mtmiller
  •  

    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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-11-11 pantxo StatusNone Fixed
        Open/ClosedOpen Closed
    2014-10-31 pantxo Attached File- Added fix_gmcall.patch, #32348
    2014-10-30 mtmiller Attached File- Added genpropdoc.diff, #32343
        Carbon-Copy- Added pantxo

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code