Jump to content
  • Advertisement
Sign in to follow this  
BlackJoker

Issue with convertion C++ code to C# for TGA loading

This topic is 851 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I want to implement TGA support for my engine based on reimplemented Sharpdx Toolkit, so I went here: http://directxtex.codeplex.com/SourceControl/latest#DirectXTex/DirectXTexTGA.cpp and tried to convert that code to C# with ShaprdDX specific classes like Image to load TGA format, but I faced with problem in part where I need to uncompress pixels.

Seems I am missing something, because during execution that code reach coditions that it should not reach actually.

 

Here is my implementation in C#:

For test purpose I add only one codition when texture is 32 bit per pixel.

private static unsafe Image CreateImageFromTGA(IntPtr pTGA, int offset, int size, ImageDescription description,
         TGAConversionFlags convFlags, GCHandle? handle)
      {
         int rowPitch = 0;

         var isCopyNeeded = ((convFlags & (TGAConversionFlags.Expand|TGAConversionFlags.RLE)) != 0);

         if ((convFlags & TGAConversionFlags.Expand) != 0)
         {
            rowPitch = description.Width*3;
         }
         else
         {
            int slicePitch = 0;
            int newWidth;
            int newHeight;
            Image.ComputePitch(description.Format, description.Width, description.Height, out rowPitch, out slicePitch, out newWidth, out newHeight);
         }


         Image image = new Image(description, pTGA, offset, handle, !isCopyNeeded);

         var pixelBuffer = image.PixelBuffer[0];

         byte* sPtr = (byte*) pixelBuffer.DataPointer;
         //byte* endPtr = (sPtr + (size - offset));
         var endPtr = sPtr + pixelBuffer.BufferStride;

         
         //If TGA compressed, then we must uncompress pixels
         if (convFlags.HasFlag(TGAConversionFlags.RLE))
         {
            switch (description.Format)
            {
               case Format.R8G8B8A8_UNorm:
                  bool nonzeroa = false;
                  for (int y = 0; y < description.Height; ++y)
                  {
                     int _offset = convFlags.HasFlag(TGAConversionFlags.InvertX) ? description.Width - 1 : 0;

                     uint* dstPtr = (uint*) (((byte*) pixelBuffer.DataPointer) +
                                             (pixelBuffer.RowStride*
                                              (convFlags.HasFlag(TGAConversionFlags.InvertY)
                                                 ? y
                                                 : (description.Height - y - 1))) +
                                             _offset);

                     for (int x = 0; x < description.Width;)
                     {
                        if ((*sPtr & 0x80) == 0x80)
                        {
                           int j = (*sPtr & 0x7F) + 1;
                           ++sPtr;

                           uint t;
                           if (convFlags.HasFlag(TGAConversionFlags.Expand))
                           {
                              if (sPtr + 2 >= endPtr)
                              {
                                 return null;
                              }

                              // BGR -> RGBA
                              t = (uint) ((*sPtr << 16) | (*(sPtr + 1) << 8) | (*(sPtr + 2)) | 0xFF000000);

                              nonzeroa = true;
                           }
                           else
                           {
                              if (sPtr + 3 >= endPtr)
                              {
                                 return null;
                              }

                              // BGRA -> RGBA
                              t = (uint) ((*sPtr << 16) | (*(sPtr + 1) << 8) | (*(sPtr + 2)) | (*(sPtr + 3) << 24));

                              if (*(sPtr + 3) > 0)
                              {
                                 nonzeroa = true;
                              }

                              sPtr += 4;
                           }

                           for (; j > 0; --j, ++x)
                           {
                              if (x >= description.Width)
                              {
                                 return null;
                              }

                              *dstPtr = t;

                              if (convFlags.HasFlag(TGAConversionFlags.InvertX))
                              {
                                 --dstPtr;
                              }
                              else
                              {
                                 ++dstPtr;
                              }
                           }
                        }
                        else
                        {
                           var j = (*sPtr & 0x7F) + 1;
                           ++sPtr;

                           if (convFlags.HasFlag(TGAConversionFlags.Expand))
                           {
                              if (sPtr + (j*3) > endPtr)
                              {
                                 return null;
                              }
                           }
                           else
                           {
                              if (sPtr + (j*4) > endPtr)
                              {
                                 return null;
                              }
                           }

                           for (; j > 0; --j, ++x)
                           {
                              if (x >= description.Width)
                              {
                                 return null;
                              }

                              if (convFlags.HasFlag(TGAConversionFlags.Expand))
                              {
                                 if (sPtr + 2 >= endPtr)
                                 {
                                    return null;
                                 }

                                 //BGR -> RGBA
                                 *dstPtr = (uint) ((*sPtr << 16) | (*(sPtr + 1) << 8) | (*(sPtr + 2)) | 0xFF000000);
                                 sPtr += 3;

                                 nonzeroa = true;
                              }
                              else
                              {
                                 if (sPtr + 3 >= endPtr)
                                 {
                                    return null;
                                 }

                                 // BGRA -> RGBA
                                 *dstPtr =
                                    (uint) ((*sPtr << 16) | (*(sPtr + 1) << 8) | (*(sPtr + 2)) | (*(sPtr + 3) << 24));

                                 if (*(sPtr + 3) > 0)
                                 {
                                    nonzeroa = true;
                                 }

                                 sPtr += 4;
                              }

                              if (convFlags.HasFlag(TGAConversionFlags.InvertX))
                              {
                                 --dstPtr;
                              }
                              else
                              {
                                 ++dstPtr;
                              }
                           }
                        }
                     }

                     if (!nonzeroa)
                     {
                        SetAlphaChannelToOpaque(image);
                     }
                  }
                  break;
            }
         }

         return image;
      }

And my code reach the following condition:

for (; j > 0; --j, ++x)
    {
      if (x >= description.Width)
        {
           return null;
        }
.....

At some reason my x variable reach texture width, but at the same time my j variable is not 0 at that moment.

Seems I have missed something during pointer calculations, but I dont see what exactly.

Could someone help me with this please?

 

Share this post


Link to post
Share on other sites
Advertisement
From the TGA spec: "The RLE packet may cross scan lines"

So, that might explain why you still have some pixels left in a run (j variable) when you reach the width of an image.

I would change the loop so that it loops over the file data (two loops - 'while more data in file' and 'while more pixels from RLE packet') and adjusts the image position appropriately, instead of nesting the data loop inside width/height loops at all. Edited by Nypyren

Share this post


Link to post
Share on other sites

The first thing is that in MS sources for TGA loading you can see that RLE flag will be set to the data even if it actually has no compression.

 

And I am not actually understand how/why MS code working in this case? I dont think they upload non-working solution.

Share this post


Link to post
Share on other sites
If you tested the C++ code with the image you're using, and it works, then the image isn't relying on cross-scan packets (programs that save the TGA files sometimes intentionally split runs when the end of a scanline is reached). In that case there's probably a bug in your transliteration of the C++ code.

The main gotcha that I have run into when transliterating C++ -> C# is the differences in implicit casting between different sizes of integers that C# uses.

What I would do next is run the C++ and C# code in debuggers and step through them in parallel with the same TGA file until you see where values diverge. Edited by Nypyren

Share this post


Link to post
Share on other sites

I think I have found the root cause of the crashing:

else
     {
       int j = (*sPtr & 0x7F) + 1;
       ++sPtr;

Here I take byte value from the memory and add one.

 

Similar string in C++ looks this:

else
   {
    // Literal
    size_t j = (*sPtr & 0x7F) + 1;
    ++sPtr;

For ex., if byte value for particular memory cell is less than 0x7F (for ex. 35), then you will have j = 35+1 = 36. In the same time width of your texture is 10 pixels. So, you will end up returning null, because the following condition will always true:

for (; j > 0; --j, ++x)
    {
      if (x >= description.Width)
       {
          return null;
       }

So, obviously, I translated C++ to C# incorrectly, but how to do it in correct way?

Share this post


Link to post
Share on other sites

Here is part of my TGA decoder, I handle all the different TGA modes so each decoder is a separate sub-routine.

 

You don't need to use unsafe code and if speed is an issue, read the whole file into memory and create a BinaryReader from a MemoryStream.

 

If you want to grab the whole TGA loader, it is part of the source code available here http://stainlessbeer.weebly.com/downloads.html

 

It's in the Il2modder source code

// run length encoded 32 bit
        private void RlcTrueColour32(BinaryReader b)
        {
            int size = ImageWidth * ImageHeight;
            int pos = 0;
            int wp = 0;
            while (pos < size)
            {
                byte h = b.ReadByte();
                if (h < 128)
                {
                    h++;
                    while (h > 0)
                    {
                        ImageData[wp++] = b.ReadByte();
                        pos++;
                        h--;
                    }
                }
                else
                {
                    h -= 127;
                    byte p = b.ReadByte();

                    for (int i = 0; i < h; i++)
                    {
                        ImageData[wp++] = p;
                        pos++;
                    }

                }
            }
        }
Edited by Stainless

Share this post


Link to post
Share on other sites
byte* sPtr = (byte*) pixelBuffer.DataPointer;
uint* dstPtr = (uint*) (((byte*) pixelBuffer.DataPointer) +

OK, I think I see the major problem; It looks like you're reading and writing the same buffer simultaneously. That'll definitely not work with TGA files. You'll start getting corruption the moment the write pointer writes to data that the read pointer is about to read.

You definitely need to have the writes not interfere with reads.

Personally I would use a BinaryReader like Stainless' TGA reader does, since it's clean and looks more like usual C# code. Edited by Nypyren

Share this post


Link to post
Share on other sites

Thanks for help. I had some more problems with by code, but eventually I fixed them and now can load RLE/Expand tga textures as well as non RLA, but one issue is still left.

 

I need to create another instance of Image class to have some destination where I can write to. After all operations finished, I want to copy memory from one location to another using 

Utilities.CopyMemory(image.DataPointer, imgDst.DataPointer, image.PixelBuffer[0].BufferStride);

But problem is that if texture has Expand flag, this function will throw Access violation exception. If texture has no Expand flag, I could copy it without problems.

In general I could just return new image and dispose old, but I am just wondering is it possible just to copy destination image to source in my case?

Edited by BlackJoker

Share this post


Link to post
Share on other sites
Why do you store the TGA file's data in an Image at all? It's not an image, it's a file.

Why do you have pTGA in an IntPtr? Why is it not a (File)Stream, BinaryReader, or at very least a byte[]?

Can you show the code where that IntPtr is coming from? Generally you never have to use IntPtrs in C# unless you're doing interop, and I don't see why you need any interop when reading from a file. Edited by Nypyren

Share this post


Link to post
Share on other sites

1. Image - its Shaprdx class for storing information and datapointers to the graphics files such as png, bmp, dds and other file formats. 

I dont see any reason to not using it, because later I am creating D3D texture from data, calculated inside Image class.

 

2. Here is code for loading from stream:

NativeFileStream stream = null;
            IntPtr memoryPtr = IntPtr.Zero;
            int size;
            try
            {
                stream = new NativeFileStream(fileName, NativeFileMode.Open, NativeFileAccess.Read);
                size = (int)stream.Length;
                memoryPtr = Utilities.AllocateMemory(size);
                stream.Read(memoryPtr, 0, size);
            }
            catch (Exception)
            {
                if (memoryPtr != IntPtr.Zero)
                    Utilities.FreeMemory(memoryPtr);
                throw;
            }
            finally
            {
                try
                {
                   stream?.Dispose();
                }
                catch {}
            }

            // If everything was fine, load the image from memory
            return Load(memoryPtr, size, false);

As you can see pTGA - its pointer to the memory where whole file is stored. So, it is actually stream data.

 

Update:

after looking to the DDS loader code, I see that there is no need to copy one image to another. Just return new image as I did. So, this question can be closed, I think.

Edited by BlackJoker

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!