STR #2873: Fl_JPEG_Image: when file not found, d() returns 3 instead of 0

STR #2873

Application:	FLTK Library
Status:	1 - Closed w/Resolution
Priority:	3 - Moderate, e.g. unable to compile the software
Scope:	3 - Applies to all machines and operating systems
Subsystem:	Image Support
Summary:	Fl_JPEG_Image: when file not found, d() returns 3 instead of 0
Version:	1.3-current
Created By:	greg.ercolano
Assigned To:	matt
Fix Version:	1.3.4 (SVN: v10732)
Update Notification:	Receive EMails Don't Receive EMails

Trouble Report Files:

#1	greg.ercolano 06:08 Sep 20, 2012

foo.cxx
1k

Trouble Report Comments:

#1	greg.ercolano 06:08 Sep 20, 2012

The docs for the Fl_JPEG_Image ctor says: > There is no error function in this class. > If the image has loaded correctly, > w(), h(), and d() should return values greater zero. <-- However, I just did a test using the attached (foo.cxx); if the jpg file doesn't exist, d() should return zero but it does not. (in my test, it returns 3) Logging this as a bug, since it disagrees with the docs. FWIW, testing w() & h() for zero works. Suggest that if the file can't be opened, the code explicitly sets w()/h()/d() to zero to comply with the docs, and not rely on the subclass ctors to zero these out. Also suggest testing other image types (PNG, BMP, etc)

#2	AlbrechtS 08:31 Oct 19, 2012

Hmm, I believe this is not a bug. As you cited from the docs, "If the image has loaded correctly, w(), h(), and d() should return values greater zero", this doesn't mean that ALL values must be zero if it failed. The correct negation of this condition is: "If the image has NOT loaded correctly, AT LEAST ONE OF w(), h(), and d() should return a value (less than or) equal (to) zero". Maybe it would be better to document the latter case, or even only that the image has not loaded correctly, if either w() or h() are zero, which means in fact a zero size image - d() wouldn't be relevant, anyway. Close STR, update docs accordingly? Greg, or anybody else?

#3	greg.ercolano 09:09 Oct 19, 2012

OK: assigning this to myself and will solve in docs. A small bit of example code should remove any ambiguity: if ( img->w() <= 0 || img->h() <= 0 || img->d() <= 0 ) { ..an error occurred.. } I think I'll avoid the specifics of error handling (eg. using strerror()/perror() to access file system errors) because the failure might be caused by a non-operating system issue (e.g. bad values in the image header) where any value of errno would be misleading.

#4	greg.ercolano 18:08 Oct 19, 2012

It looks like /only/ Fl_JPEG_Image documents this behavior; the others (Fl_BMP_Image, Fl_Shared_Image, etc) don't. I'm going to try to check their behaviors empirically to see if they behave similarly and document it, so that if they /don't/ behave that way, it'll be listed as a bug. I'm assuming it's correct to have the docs assume all the Fl_XXX_Image classes should handle errors the same way; if all of x/y/d are <= 0 then an error occurred.

#5	AlbrechtS 02:33 Oct 20, 2012

I assume the last sentence has some typos: "all of x/y/d are <= 0" should read "any of w/h/d is/are <= 0" ? .. or similar. And to the question: yes, I think they should all behave the same, since they can all be loaded by Fl_Shared_Image transparently (i.e. independent of the particular image type).

#6	ianmacarthur 16:10 Mar 11, 2014

Status on this?

#7	greg.ercolano 16:35 Mar 11, 2014

Never got around to testing the different image formats for file not found/permission error conditions to see the results of w/h/d. I really wish we had something like a 'fail()' method test, like 'if ( image.fail() )' so apps don't have to check w/h/d, and a clear way of passing back OS errors (vs. non-OS errors)

#8	matt 16:44 May 23, 2015

new function Fl_Image::fail() that returns 0, ERR_NO_IMAGE, ERR_FORMAT, or ERR_FILE_ACCESS to make life easier when loading images. Implemented for all image formats. Should be ABI compatible by re-using ld_ to store an error code.

#9	greg.ercolano 10:22 May 24, 2015

Thanks Matt! Made a small mod as r10733 to fix build errors under Linux..

#10	greg.ercolano 10:26 May 24, 2015

Changing the fix# from 10733 -> 10732, which is matt's actual fix release. Not sure how it was set to 10733 -- wasn't me, even though I made a small fix in 10733.

#11	matt 11:08 May 24, 2015

I was foreseeing that you would fix my code, so I chose a higher number (10733). Actually, Xcode had shown me the current number *after* the commit, but I thought it was still the number before the commit. Thanks.

#12	greg.ercolano 11:57 May 24, 2015

Cool -- didn't know Xcode had hooks into SVN. I'm interested in your fix because I intend to use it in all my apps; always wanted this fail() check..! BTW, I added a code example to fail() in r10734; you might want to check it. Wanted to show proper use to app programmers. Feel free to apply changes. (I also moved the docs for fail() from .H -> .cxx so that the docs are near the code implementation)