SFML community forums
General => Feature requests => Topic started by: JayArby on April 14, 2011, 05:00:43 pm
-
I think resources, such as Images, should be unloadable.
myImage.Destroy();
or something like that. Otherwise, you basically have to leave unneeded garbage in memory, use pointers, or else make sure that all resources go out of scope at exactly the right time, which means stack allocated static global objects are no good.
I already added unload functions to some resources on my computer, but it would be nice if SFML would come with this functionality. I can't understand why it doesn't.
My general rule is, if you are able to have empty objects at any time, then they should provide a public function to reset them. It doesn't make sense (to me) to allow construction of uninitialized objects but never allow the resource to be freed after initialization.
-
myImage = sf::Image();
-
Well if you tell them to load a new image for instance I think the old memory is free'd and replaced with the new data.
Also you can control when the memory is free'd using:
sf::Image *image = new Image();
/* Do stuff with image */
delete image;
-
SFML hasn't got any resource manager. Hence a resource could not be destroyed or unloaded.
I would probably hate the SFML, if there were functions like:
image.Destroy()
The image would exist further on, but there would not be any valid image information. So have fun with runtime errors ;).
If you want to unload an image, you have to use an image manager which loads and unloads images. In this manner you make sure that an image is deleted from memory and not usable anymore.
-
yes, but there seem to be two conflicting philosophies in SFML. If I declare an object like this:
sf::Image myImage;
I have an empty, uninitialized object. So it IS possible to have dead objects.
But after something has been loaded, you can never free the object again. It's like an odd combination of the Java philosophy with the C++ philosophy. If you are going to ensure that the programmer cannot have empty objects, then the above constructor should be disabled.
Either allow empty objects, or disallow them--don't allow only part of the time.
-
Laurent has a good point, but it seems like a clumsy workaround to me.
It is pretty easy to add "Unload" methods to resource objects. As I said, I have already done it myself. I don't see why we should use "image = sf::Image();" rather than the more obvious form, "image.Unload();"
-
The possibility of having empty objects, means not, that empty objects are good.
A sf:Image is supposed to contain image information. So you should load your image as soon as possible(to prevent errors). And if you don't need the image anymore, you should destroy the variable(e.g. removing it from a list) and not just remove the image information.
In my opinion it is a design problem. You should not create variables and if you don't need them anymore just reset them(and risk to use them although you mustn't).
But if you need it, just use pointers. These will do exactly what you want to. So why not?
And of course it would be pretty easy to add such a function, but - in my opinion - the user is not supposed to do that. And you shouldn't implement functions which are not recommendable to use ;)
-
Laurent has a good point, but it seems like a clumsy workaround to me.
It is pretty easy to add "Unload" methods to resource objects. As I said, I have already done it myself. I don't see why we should use "image = sf::Image();" rather than the more obvious form, "image.Unload();"
It makes sense.
However I'm not sure it is really necessary; maybe you should give us an example of a situation where unloading an image while keeping the instance alive is really useful.
-
There's no clean solution to this, I think. What'd be nice is if sf::Image had no constructor but a static Create() method that creates an object for you and also initializes it (by width, height and color).
To load images, you'd use an image loader. AFAIR Laurent had something like that on the roadmap, i.e. moving LoadFromFile() away from sf::Image and provide an external interface to load from different file formats.
That way you won't have uninitialized (or invalid) sf::Image objects anymore.
-
AFAIR Laurent had something like that on the roadmap, i.e. moving LoadFromFile() away from sf::Image and provide an external interface to load from different file formats.
Really? :lol:
I remember talking about image formats plugins, but no change in the public API of sf::Image.
That way you won't have uninitialized (or invalid) sf::Image objects anymore.
I wouldn't call them "uninitialized" or "invalid", but "empty". they are perfectly valid, it's just that they contain no pixel data.
Avoiding them is not possible anyway, unless I use exceptions.
-
What'd be nice is if sf::Image had no constructor but a static Create() method that creates an object for you and also initializes it (by width, height and color).
And what do you want to return?
- sf::Image? Possibly an unnecessary expensive copy. Move-semantics would help, but SFML doesn't use C++0x.
- sf::Image*? Possibly an unnecessary heap allocation and certainly unnecessary manual memory management.
-
sf::Image? Possibly an unnecessary expensive copy. Move-semantics would help, but SFML doesn't use C++0x.
If I add sf::Image::Swap you can use the following trick:
sf::Image image;
sf::Image::Load("...").Swap(image);
Not very sexy, I admit ;)
-
Yes, but then you can as well write this :)
sf::Image image;
image.Load("...");
-
True :D
What'd be nice is if sf::Image had no constructor but a static Create() method that creates an object for you and also initializes it (by width, height and color).
The problem here is "no constructor". It would make the class hardly usable. Imagine the following situation:
class Game
{
public:
Game()
: tileset(?????)
{
}
void LoadMap(const std::string& tileset)
{
// too late for loading the image...
}
private:
sf::Image tileset;
};
-
I wouldn't call them "uninitialized" or "invalid", but "empty".
Then sf::Image::Unload() or sf::Image::Clear() would just be fine. To be honest I can't really think of situations where it's really needed, but for consistency I'd say it's okay.
And what do you want to return?
bool Load( const std::string& filename, sf::Image& image );
I remember talking about image formats plugins, but no change in the public API of sf::Image.
Sorry, that's what I meant. Can't reach the old roadmap anymore, but of course it would also work with the current implementation. If writing plug-ins means to provide an external function that drops an sf::Image, there doesn't need to be changed anything in SFML, at all. ;) That's why I thought the API would change slightly.
-
bool Load( const std::string& filename, sf::Image& image );
Hm, that's not what I understood by "static create method" ;)
I don't see the advantage of this function over the current
sf::Image;
Image.LoadFromFile("image.bmp");
A public default constructor is still necessary (or what do you pass to the sf::Image& parameter?), you cannot prevent empty objects, besides the signature is unintuitive because the (quasi) this object is not the first parameter.
-
I wouldn't call them "uninitialized" or "invalid", but "empty".
Then sf::Image::Unload() or sf::Image::Clear() would just be fine. To be honest I can't really think of situations where it's really needed, but for consistency I'd say it's okay.
I don't like the word "consistency", because if a function is not needed you should not implement it in my opinion. The very simple reason is, that it is easy to use it in a wrong way. You would be able to clear an image, but you also would be able, to "use" this empty image again. And so of course an image can be empty, but it isn't supposed to.
I rather would prefer not to offer any default constructor. So that the user had to initialise the image immediately after creating it.
If you cant initialise it immediately(see Laurent's example) I would use pointers(I would use them even with a default constructor, because it is very simple to check, weather a pointer is NULL or not and so you can prevent missuse of an uninitialised image).
So the code would look like:
class Game
{
public:
Game()
: tileset()
{
}
~Game()
{
if( tileset )
delete tileset;
}
void LoadMap(const std::string& tileset)
{
if( !tileset )
tileset = new sf::Image( "myImage.jpg" );
}
private:
sf::Image* tileset;
};
-
If you cant initialise it immediately(see Laurent's example) I would use pointers(I would use them even with a default constructor, because it is very simple to check, weather a pointer is NULL or not and so you can prevent missuse of an uninitialised image).
You just introduced unnecessary dynamic memory allocation.
Honestly, I can't see how not having a default constructor could be better than having one which creates an empty image, and I can't see why a Clear() method would be so evil. Okay, it's possible to use an empty image, either by accident or intentionally. But think about it: in this regard, built-in types are much worse. You can use uninitialized int. You can use uninitialized pointer. The effect is quite bad: you end up with undefined behavior. On the other hand, at least some operations are well-formed for an empty image.
-
Honestly, I can't see how not having a default constructor could be better than having one which creates an empty image
An empty image can be meaningful, this also has to do with Laurent's "policy" that image loading errors are no fatal errors (e.g. show a white sprite instead). But generally, it's certainly not bad to ensure valid instances when possible. This saves the programmer from logic errors and case differentiations.
and I can't see why a Clear() method would be so evil.
I think the question is rather if a Clear() method is really useful. There are hundreds of features which are not evil, yet which don't deserve an introduction into SFML because of their irrelevance. And I haven't ever felt the need for a sf::Image::Clear() function, to be honest.
But think about it: in this regard, built-in types are much worse.
True, but that's not really an argument. There is always something worse. Apart from that, sf::Image has much higher level semantics than built-in types, therefore it should provide more restrictions about value semantics and object validity.
-
I think the question is rather if a Clear() method is really useful.
I was responding to Fred_FS and since he seems to claim that it'd be evil, based on the fact it's possible to misuse it (heck, it's C++, it's possible to misuse pretty much everything!), I wonder if it really is.
And if someone needs it, for a reason I can't think of, it exists (http://sfml-dev.org/forum/viewtopic.php?p=30125#30125), it's only spelled differently ;)
-
I am sorry. I did not know that SFML doesn't crash, if you try to use an "empty" image(I never tried it ;)). Hence it wouldn't be that bad to use a cleared image.
And of course the user is able to misuse almost everything in C++, but that is not a reason to offer functions that are unnecessary and risky.
My point is that if a user want to use such a function his design is probably imperfect. Just deleting some image information, but keeping an empty variable, is in my eyes bad design. So I just want to warn not to change a system that it meets the needs, but to consider weather the own needs are sensible.
And for this reason I would not provide a function to clear an image, because in many cases it would be misused(in that way that it is used in a bad design).
-
Sounds like a lot of you are coming from the Java viewpoint, especially with those static factory methods.
In Java, all objects are pointers and null pointers are basically empty objects. In C++, it is completely normal to have stack allocated objects that are invalid until initialized. It's no different from Java in substance, because rather than testing for a null pointer, you convert the object to bool or call some method to test it, like file.isOpen();
Again, I'm fine with the Java way or the C++ way, but mixing them doesn't seem like a good idea.
It makes sense.
However I'm not sure it is really necessary; maybe you should give us an example of a situation where unloading an image while keeping the instance alive is really useful.
Well, here is my personal example: a static Image object is used to display a thumbnail view of the currently selected image in a list of image files. When no image is selected, there is no reason to keep the image in memory. So I want to unload the image so that the thumbnail display will automatically know, on checking the image, that nothing needs to be displayed. So I had to use pointers. No big deal, but it just doesn't seem like the Image class was really meant to be used like that. It would have been more elegant to use a stack allocated object.
-
In C++, it is completely normal to have stack allocated objects that are invalid until initialized.
Certainly not, invalid objects are almost always bad programming style. In constrast to Java, C++ is able to enforce the initialization of member variables, which is a good thing. As mentioned, an empty sf::Image is not strictly invalid in this sense.
I know that some people use Init() methods instead of constructors and have status flags and case differentiations, but that's neither normal nor the good way to go.
So I had to use pointers.
You could use the assignment of an empty image.
-
Sounds like a lot of you are coming from the Java viewpoint, especially with those static factory methods.
Are you talking with me? I am certainly not coming from Java. But I also like the way to use constructors to initialise my objects ;).
Again, I'm fine with the Java way or the C++ way, but mixing them doesn't seem like a good idea.
C++ offers pointer. So you aren't mixing anything. Avoiding dynamic memory allocation doesn't mean not to use it at all.
Well, here is my personal example: a static Image object is used to display a thumbnail view of the currently selected image in a list of image files. When no image is selected, there is no reason to keep the image in memory. So I want to unload the image so that the thumbnail display will automatically know, on checking the image, that nothing needs to be displayed. So I had to use pointers. No big deal, but it just doesn't seem like the Image class was really meant to be used like that. It would have been more elegant to use a stack allocated object.
I would prefer to keep the image in memory because unloading and loading images is probably not the best for your performance.
another way is to use an image loader. This image loader has a list, with all images, and if you need an image it checks if it is necessary to load the image or if the image is already in the list. If you don't need an image anymore, you can simply remove it from this list and you don't have any empty images.
but as I have already mentioned: I don't think that it is the fastest way, to unload and reload the images.
-
Maybe "uninitialized" is the wrong word. "Empty" is a better word. It doesn't imply that the data wasn't initialized.
Example:
fstream myfile; // <- empty object
...
myfile.close(); // <- empty object
-
so...I presume the answer to my feature request is 'no'? :)
-
Absolutely ;)