Ogre::Image save to memory

What it says on the tin: a place to discuss proposed new features.
Post Reply
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Ogre::Image save to memory

Post by CABAListic »

I'm not exactly sure what status the Image class has (convenience, utility, dependency, ...?), but I needed to create a few small preview versions of some of my textures, and I figured since it's there I can just as well use Ogre::Image for the task. My only "problem" is that Image only offers a save function to write to file by passing a filename. This is inconvenient for me since I'm using PhysFS for my file management and therefore do not have immediate access to the actual physical filename where I want to store the result. I can work around it, so it's really far from being any serious concern, I just thought it would be cool if there was a more "flexible" save method.

If there's no argument against this (and I find some spare minutes), I might even have a go at it, doesn't look too complicated. Only question is how the signature for the added save method should look. I would favor the following:

Code: Select all

void save(const String& extension, std::ostream& output);
but this might be more "true" to Ogre's internals:

Code: Select all

DataStreamPtr save(const String& extension);
Or should both be added? After all, the latter just seems like a pass-through to Codec::code, with the former becoming a wrapper around it.
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 67
Contact:

Post by sinbad »

Codec::code can do this - you can't do it to an output stream really because the size of the result cannot be known from outside. Image::save returning a DataStreamPtr is the only realy way to do it but as an interface it doesn't feel 'right' to me. Maybe if it was called Image::encode...
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Post by CABAListic »

Why would the output stream need to know the size beforehand? I would simply read from the DataStream returned from Codec::code as long as it holds data and transfer it to the ostream (which might be ofstream, ostringstream or my own ostream based class).
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 67
Contact:

Post by sinbad »

Because the stream is just an intermediate mechanism - you're talking about 'to memory' so the stream would have to point to an area of memory, the size of which you can't know without invoking the compression anyway. You could assume it'll be smaller than the uncompressed size and just waste some space but it's more efficient to get the code() implementation to allocate it.

[edit]I was referring to why save(const String& extension, std::ostream& output) wasn't appropriate, maybe there's been some confusion here[/edit]
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Post by CABAListic »

You're right, to get the encoded image in a fixed memory block, the DataStream is probably the only sensible way. Calling it encode or whatever doesn't really matter to me :)
I'd still find an ostream wrapper useful, though. With boost::iostreams around, ostream could potentially open writing to numerous other custom-made streams. For example, my PhysFS wrapper has its own FileStream for file access, and I could immediately pass it to such an Image::save function without having to write any extra translation code.
But as I said, it's not important.
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Post by CABAListic »

Just had a go at it, since it's really so straight-forward. I suppose the ostream version is actually not really "needed" since the code it encapsulates is pretty much a one-liner itself, so it's more a "for the eye" wrapper than anything else. It's included in this draft, but I could remove it again.

Code: Select all

Index: OgreMain/include/OgreImage.h
===================================================================
RCS file: /cvsroot/ogre/ogrenew/OgreMain/include/OgreImage.h,v
retrieving revision 1.49
diff -u -r1.49 OgreImage.h
--- OgreMain/include/OgreImage.h	31 Aug 2006 22:47:53 -0000	1.49
+++ OgreMain/include/OgreImage.h	4 Sep 2007 15:16:35 -0000
@@ -34,6 +34,8 @@
 #include "OgrePixelFormat.h"
 #include "OgreDataStream.h"
 
+#include <iosfwd>
+
 namespace Ogre {
 
     enum ImageFlags
@@ -288,6 +290,20 @@
         */
         Image & load(DataStreamPtr& stream, const String& type );
         
+        /** Encode the image to a DataStream.
+            @param
+                type The type you want to encode the image as.
+        */
+        DataStreamPtr encode(const String& type);
+        
+        /** Save the image to a std::ostream.
+            @param
+                stream The ostream to write to.
+            @param
+                type The type you want to save the image as.
+        */
+        void save(std::ostream& stream, const String& type);
+        
         /** Save the image as a file. */
         void save(const String& filename);
 

Code: Select all

Index: OgreMain/src/OgreImage.cpp
===================================================================
RCS file: /cvsroot/ogre/ogrenew/OgreMain/src/OgreImage.cpp,v
retrieving revision 1.64
diff -u -r1.64 OgreImage.cpp
--- OgreMain/src/OgreImage.cpp	2 Jan 2007 16:33:59 -0000	1.64
+++ OgreMain/src/OgreImage.cpp	4 Sep 2007 15:16:20 -0000
@@ -388,7 +388,47 @@
 
 		pCodec->codeToFile(wrapper, filename, codeDataPtr);
 	}
-	//-----------------------------------------------------------------------------
+
+  //-----------------------------------------------------------------------------
+  DataStreamPtr Image::encode(const String& type)
+  {
+    if( !m_pBuffer )
+    {
+      OGRE_EXCEPT(Exception::ERR_INVALIDPARAMS, "No image data loaded",
+        "Image::encode");
+    }
+
+    Codec* pCodec = Codec::getCodec(type);
+    if( !pCodec )
+      OGRE_EXCEPT(
+      Exception::ERR_INVALIDPARAMS,
+      "Unable to encode image - invalid extension.",
+      "Image::encode" );
+
+    ImageCodec::ImageData* imgData = new ImageCodec::ImageData();
+    imgData->format = m_eFormat;
+    imgData->height = m_uHeight;
+    imgData->width = m_uWidth;
+    imgData->depth = m_uDepth;
+    // Wrap in CodecDataPtr, this will delete
+    Codec::CodecDataPtr codeDataPtr(imgData);
+    // Wrap memory, be sure not to delete when stream destroyed
+    MemoryDataStreamPtr wrapper(new MemoryDataStream(m_pBuffer, m_uSize, false));
+
+    return pCodec->code(wrapper, codeDataPtr);
+  }
+
+  //-----------------------------------------------------------------------------
+  void Image::save(std::ostream& stream, const String& type)
+  {
+    // encode to memory
+    DataStreamPtr data = encode(type);
+
+    // write results to the stream
+    stream << data->getAsString();
+  }
+	
+  //-----------------------------------------------------------------------------
 	Image & Image::load(DataStreamPtr& stream, const String& type )
 	{
 		if( m_pBuffer && m_bAutoDelete )
If that's acceptable, I'll submit it properly.

BTW, since I'm just touching the Image class, I have two more "issues" with the load functions. For one, the two load functions (one taking name and group to load from the resource system, the other to load from a DataStream) share a lot of code - actually I think the former should call the other, since it retrieves a DataStream from the ResourceGroupManager.
And then, correct me if I'm wrong, but I had the impression that whenever I access a resource from Ogre's resource system, specifying the group name was optional. If the group name is omitted, it would just search the standard group first and then extend the search to other groups. However, the Image load function is an exception since the group parameter has no default value. Is this intentional, or could the group parameter get a default value of ResourceGroupManager::DEFAULT_RESOURCE_GROUP_NAME? Again no big issue, I just stumble over it occasionally :)
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 67
Contact:

Post by sinbad »

Ah, I actually committed an implementation earlier today :)

I can't recall the reasons for not defaulting in every case, I thought there was one but it eludes me - sorry, on the run right now so I'll have to come back to that.
CABAListic
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 2903
Joined: Thu Jan 18, 2007 2:48 pm
x 58
Contact:

Post by CABAListic »

Oh, alright. All the better :)

Edit: One more issue (sorry ;) ). Shouldn't Image::getColourAt be const?
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 67
Contact:

Post by sinbad »

Yeah. Will change these things in Shoggoth to avoid breaking changes.
Post Reply