Blit Subtextures from memory

[tuan kuranes]

I'm trying to update only part of textures, but cannot achieve any good result (black textures in GL and Exception in DX9)...

Here's my code :

Code: Select all

// try to upload only the smallest rectangle containing modification
// here just upload half the texture data
PixelBox src;

src =  mImage.getPixelBox();
src.right = part.right/2;
src.bottom = part.bottom/2;
src.left = part.left;
src.top = part.top;
src.front = part.front;
src.back = part.back;

Image::Box destbox;

destbox.right = src.right;
destbox.bottom = src.bottom;
destbox.left = src.left;
destbox.top = src.top;
destbox.front = src.front;
destbox.back = src.back;

mBuffer->lock(HardwareBuffer::HBL_DISCARD);
mBuffer->blitFromMemory (src, destbox);
mBuffer->unlock();  

[:wumpus:]

Code: Select all

// try to upload only the smallest rectangle containing modification
// here just upload half the texture data
PixelBox src;

src =  mImage.getPixelBox();
src.right = part.right/2;
src.bottom = part.bottom/2;
src.left = part.left;
src.top = part.top;
src.front = part.front;
src.back = part.back;

This doesn't update the src.data value; you need to get a subregion of an image with .getSubVolume()

Code: Select all

Image::Box srcBox;
srcBox.right = part.right/2;
srcBox.bottom = part.bottom/2;
srcBox.left = part.left;
srcBox.top = part.top;
srcBox.front = part.front;
srcBox.back = part.back;

Image::Box dstBox = srcBox;

mBuffer->blitFromMemory (src.getSubVolume(srcBox), dstBox);

And you should never have any of the buffers locked when doing blitFromMemory. The lock() and unlock() methods are there so you can write directly to the underlying memory without a function like blitFromMemory. For example:

Code: Select all

Image::Box srcBox;
srcBox.right = part.right/2;
srcBox.bottom = part.bottom/2;
srcBox.left = part.left;
srcBox.top = part.top;
srcBox.front = part.front;
srcBox.back = part.back;

Image::Box dstBox = srcBox;

PixelBox lock = mBuffer->lock(dstBox);
// lock.data can now be freely accessed
PixelUtil::bulkPixelConversion(src.getSubVolume(srcBox), lock);

mBuffer->unlock();

[tuan kuranes]

Thanks a million !
I had hard time trying to understand how to do so.
So here's final version :

Code: Select all

const divider = 8;
const Image::Box rect(0,0,mImage.getWidth() / size,mImage.getHeight() / 8); 
PixelBox lock = mBuffer->lock(rect, HardwareBuffer::HBL_NORMAL); 
// lock.data can now be freely accessed 
PixelUtil::bulkPixelConversion(mImage.getPixelBox().getSubVolume(rect), lock); 
mBuffer->unlock();

Works great on DX9...

but on GL, it changes the whole texture, there is black stripes separating the real data, and their size seems related to divider var (black stripe is bigger if divider = 8 than 2 )

(Tested on my ati 9600 opengl 5.1 catalyst without hardware mipmap support (that does exists undex DX... crappy gl ati drivers...) so it goes on glubuild2dmimap path.))

[:wumpus:]

If you'd like to try something, please comment out the "mGLSupport->getGLVendor() != "ATI" && " on line 294 of GLRenderSystem.cpp and try again.
Recent ATI drivers appear to have solved the automip generation issue. But I need to know which ones so I can conditionally enable it for some versions.

Maybe the GLU path doesn't cope with partial blitting very well, so we'll have to that path to blit everything nevertheless

[tuan kuranes]

It works great on Catalyst 5.1 on windows XP. (GL_VERSION = 1.5.4830)

Let's Hope the Linux driver released at the same time version does the same....

[:wumpus:]

in 1.3.4769 for Linux it works great as well, actually it has been working OK for quite a while really, it's a pity that ATI never describes what they *really* fixed in a certain version just some blablba about which game has which issue.

[tuan kuranes]

It seems there's a bug in "PixelUtil::bulkPixelConversion" when Box::top isn't equal to 0.

(all other case works, at least with 2d textures, didn't try others, and same case as before, PF_BYTE_RGB -> PF_X8R8G8B8)

As soon I use rect with a top != 0 I get corrupted memory, and when it doesn't segfault, texture data between 0 and top is corrupted...

Code: Select all

const Image::Box rect (0, 128, 0, 150, 150, 1); //works
//const Image::Box rect (10, 128, 0, 150, 150, 1); //faults

const PixelBox srcBox = mImage.getPixelBox().getSubVolume(rect);
const PixelBox lock = mBuffer->lock(rect, HardwareBuffer::HBL_NORMAL); 
// lock.data can now be freely accessed  
PixelUtil::bulkPixelConversion(srcBox, lock); 
mBuffer->unlock();

I think that it would be "template <class U> struct PixelBoxConverter " in ogrepixelconversions.h that use "rowpitch" instead of some "RowSkipBytes" near line 60-61.

As it's time to go to bed here, I won't try to patch it til tomorrow...

[tuan kuranes]

ooops, forgot to post the patch

so the bug was well hidden in the

Code: Select all

 for(size_t x=rect.left...

so that dstptr[x] was resulting in reality dataptr[rect.left * 2] accessing in PixelBoxConverter, as dstptr is already moved of rect.left (in getsubvolume, ptr is moved based on rect.top and rect.left)...

Code: Select all

const size_t k = src.right - src.left;
for(size_t z=src.front; z<src.back; z++) 
{
  for(size_t y=src.top; y<src.bottom; y++)
  {
    for(size_t x=0; x<k; x++)
    {
      dstptr[x] = U::pixelConvert(srcptr[x]);
    }
    srcptr += src.rowPitch;
    dstptr += dst.rowPitch;
    }
  srcptr += srcSliceSkip;
  dstptr += dstSliceSkip;
}    

I'll submit a patch.

But i'm afraid that while reading the code, I added some const keyword... that add speed
Too bad visual studio doesn't support restrict, I would also added it, and we would have gained much speed... )

[:wumpus:]

ouch, thanks for noticing! You'll submit a patch?

What's restrict?

[tuan kuranes]

"restrict" is a hint c++ keyword that permit developper to tell the compiler that the target of the pointer will only be accessed through that pointer so that compiler can optimize data access (it does in particular helps vectorization). It's specially useful with C++, as it helps solving the aliasing problem.
(for aliasing, memory problem of c++ and restrict you should read : http://www.gdconf.com/archives/2003/Eri ... rister.ppt )

Afaik gcc and intel compilers does support restrict, but not vc.net.
( seems that vs2005 will have support for it : http://www.virtualdub.org/blog/pivot/entry.php?id=9 )

I'm trying to submit it on sf, but it's so sloooooooowwwww, I've just finished updating my cvs... not sure I'll be able to post before the night...
(btw, are you sure you committed PF_B4G4R4A4 in ogremain, I do have it in d3d9 and gl srcs but not in ogremain ?)

[tuan kuranes]

I also get a "ssize_t" undefined in ogrezip.h and ogreDatastream.h from the CVS HEAD compilation.

(if I succeed in compiling and submitting I'm afraid that my patch will also contains the modification from my previous patch posted on sf (Image::loadDynamicImage and the "if( m_pBuffer && m_bAutoDelete )" in all image loading methods ))

[:wumpus:]

"restrict" is a hint c++ keyword that permit developper to tell the compiler that the target of the pointer will only be accessed through that pointer so that compiler can optimize data access (it does in particular helps vectorization). It's specially useful with C++, as it helps solving the aliasing problem.
(for aliasing, memory problem of c++ and restrict you should read : http://www.gdconf.com/archives/2003/Eri ... rister.ppt )

Thanks!

Afaik gcc and intel compilers does support restrict, but not vc.net.
( seems that vs2005 will have support for it : http://www.virtualdub.org/blog/pivot/entry.php?id=9 )

In that case we can just define some macro in OgrePlatform.h that gets defined to empty on non-supporting compilers.

I'm trying to submit it on sf, but it's so sloooooooowwwww, I've just finished updating my cvs... not sure I'll be able to post before the night...

I'll commit it as soon as I get it

(btw, are you sure you committed PF_B4G4R4A4 in ogremain, I do have it in d3d9 and gl srcs but not in ogremain ?)

PF_B4G4R4A4 has been removed from the entire source, as the pixel format is not supported by any render system. It has been replaced by PF_A1R5G5B5 which both GL and D3D do like. Maybe you got an in-between CVS version.

[tuan kuranes]

I try to get in touch with sf submitting page... But its 60 b/s between me and sf patch tracker... my cvs updates from sf are also very slowwwww... so compilations problem can also comes from here...

anyway, meanwhile, here's the zip I'm trying to upload since 30 mn overthere :

http://tuan.kuranes.free.fr/Imageandpixelconversion.zip

[tuan kuranes]

patch submitted : http://sourceforge.net/tracker/index.ph ... tid=302997