Short form:
Fl_Shared_Image::copy() should return the proper
type (Fl_Shared_Image* instead of Fl_Image*), and
Fl_Shared_Image::refcount() should be private or
protected.
Long form:
Problem #1:
There are two copy methods:
virtual Fl_Image *copy(int W, int H);
Fl_Image *copy() { return copy(w(), h()); }
Both are declared to return Fl_Image*. In fact they _do_ return
an Fl_Shared_Image* pointer, the cast is done implicitly:
Full code from Fl_Shared_Image.cxx:
Fl_Image *
Fl_Shared_Image::copy(int W, int H) {
Fl_Image *temp_image; // New image file
Fl_Shared_Image *temp_shared; // New shared image
// Make a copy of the image we're sharing...
if (!image_) temp_image = 0;
else temp_image = image_->copy(W, H);
// Then make a new shared image...
temp_shared = new Fl_Shared_Image();
temp_shared->name_ = new char[strlen(name_) + 1];
strcpy((char *)temp_shared->name_, name_);
temp_shared->refcount_ = 1;
temp_shared->image_ = temp_image;
temp_shared->alloc_image_ = 1;
temp_shared->update();
return temp_shared;
}
IMHO this may be unintentional (cut'n'paste problem?), but it is
documented this way. However, the user is required to release the
shared image later, and thus s/he *needs* to cast the pointer back
to an (Fl_Shared_Image*) !
Here is some (correct!) example code, posted today by Ian (I hope
you don't mind that I use it here - the texture example works
like a charm ;-) - thanks also to Greg).
Fl_Image *temp;
if (img->w() > img->h())
temp = img->copy(TEX_W, TEX_H * img->h() / img->w());
else
temp = img->copy(TEX_W * img->w() / img->h(), TEX_H);
img->release();
img = (Fl_Shared_Image *)temp;
The cast in the last line wouldn't be necessary if
Fl_Shared_Image::copy() would return what it should !
Thus my question: Should this be changed? IMHO yes, but what
about compatibility? I think that most code would compile and
work okay, even if we changed it. And I tested it with Ian's
example code after my local change: it compiled and linked
without error, as I expected, but maybe there is other code?
So here is my vote:
+1 for making Fl_Shared_Image::copy() return Fl_Shared_Image*.
-----
Problem #2:
Fl_Shared_Image::refcount() should *REALLY* be private or at least
protected! This would prevent code like this:
while (img->refcount()>0) img->release();
I found this code in Fl_Help_View.cxx :-(, and I'm looking for
a better solution, but that's another story...
For those who are not familiar with Fl_Shared_Image:
If you call release() and the refcount goes down to 0, the
Fl_Shared_Image is deleted, and thus the "img" pointer would
point to a non-existent object, and then...
My vote:
+1 for making refcount() private.
-----
I'm aware that these changes would be API (and ABI) changes, and
I'm not sure if we should do this now, but IMHO this should be
done ASAP.
And, as noted recently by someone (I don't remember who): there are
more methods in Fl_Image and friends that should be checked for
proper attributes (public, protected, private). Wouldn't this be
a good time to do it, before we release FLTK 1.3.0 ?
Comments or suggestions, please ?
Albrecht
_______________________________________________
fltk-dev mailing list
[email protected]
http://lists.easysw.com/mailman/listinfo/fltk-dev
