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
|
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.
|
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.
|
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)
|
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.
|
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"):
|
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 ...
|
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?
|
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.
|
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.
|