Thread: Extremely odd Serialization error

  1. #1
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607

    Extremely odd Serialization error



    This one has me stumped big time. I'm in the process of serializing all my dialogs and objects to disk and everything was going well until.....attempting to serialize the layers dialog information to disk.

    I have a layers dialog that tells you how many tile maps you have in the final map. The actual list box has nothing to do with the class that manages the maps except that when you click add, it tells the map manager to add a map. The map size is determined by the options dialog data members which is a member of the main frame window.

    Now when I only have 2 layers, the movement layer (always layer 0) and one draw layer (layer 1) it works fine. When you add a layer and then save it all goes to hell.

    For some reason this code is not working right when more than 2 layers are available.

    Code:
    void LayersDlg::Serialize(CArchive &ar) 
    {
       if (ar.IsStoring())
      {	
        // storing code
    
        DWORD size=LayerInfo.size();
    
        CString text;
        text.Format("NumLayers: %u",size);
        ::MessageBox(0,text,0,0);
    
        ar << size;
    
      }
        else
      {	
        // loading code
    
        DWORD temp=0;
        ar >> temp;
    
        CString text;
        text.Format("NumLayers in file: %u",temp);
        ::MessageBox(0,text,0,0);
    
    
        for (DWORD i=0;i<temp;i++)
        {
          if (i>1) AddLayer();
        }
    
      }
    
    }
    Now get this. Even when you add a map and save, the dialog box in the storing section prints out the correct value. When you then attempt to load the same file, the temp variable in the loading section prints out the horizontal size of the map, which happens to be the first piece of data in the entire project file.
    I cannot for the life of me see where the CArchive is being rest to the front of the file.

    And it is not writing the correct value. I can start from scratch with a fresh compile and the number does not change. So why is the storing section writing a totally different number than is being displayed and why does it only work with 2 layers?

    Seems I may have a serious memory problem here.

    Anyone please help.

    Add button code:
    Code:
    void LayersDlg::OnButtonAdd() 
    {
      CMainFrame *ptrFrame=(CMainFrame *)AfxGetMainWnd();
      CZeldaEditorDoc *ptrDoc=(CZeldaEditorDoc *)ptrFrame->GetActiveDocument();
    
      ptrDoc->m_pMapManager->Add();
        
      AddLayer();
    
    }
    Add layer code:
    This can be called from anywhere, even in setup. This is why the count is checked. The SetupProject() function in the main window calls this to add layers for the user. By default when the project starts, it has a move layer and one draw layer.
    Code:
    void LayersDlg::AddLayer(void)
    {
      int count=m_lstLayers.GetCount();
    
      CString text;
      if (count==1)
      {
        text="Movement";
    
      }
      else
      {
        text.Format("Layer %d",count-1);
      }
    
      LayerData *data=new LayerData;
      data->dwLayerNum=count-1;
    
      data->strName=text;
    
      //CString test;
      //test.Format("Vector size: %d",LayerInfo.size());
      //::MessageBox(0,test,0,0);
    
    
      LayerInfo.push_back(data);
      m_dwNumLayers=LayerInfo.size();
    
      m_lstLayers.InsertString(count,text);
    
      
    }
    And the map manager add code:
    Code:
    DWORD Add(void)
    {
        CMap *map=new CMap();
        map->Create(m_dwWidth,m_dwHeight);
        map->Clear(0xFFFFFFFF);
    
        MapLayers.push_back(map);
        return MapLayers.size()-1;
        }
    Map create and clear code:
    Code:
    bool Create(DWORD dwWidth,DWORD dwHeight)
    {
        m_dwSize=dwWidth*dwHeight;
        m_pMap=new DWORD[m_dwSize];
    
        //return non-zero on failure
        if (!m_pMap) return true;
    
        m_dwWidth=dwWidth;
        m_dwHeight=dwHeight;
    
        return false;
    }
      
    void Clear(DWORD value=0)
    {
      DWORD *ptrMap=m_pMap;
      DWORD dwSize=m_dwSize;
    
      _asm {
        mov   edi,[ptrMap]
        mov   ecx,[dwSize]
        mov   eax,[value]
        rep   stosd
        and   ecx,03h
        rep   stosb
      }
    }
    Sorry so long but this editor is really really big and a lot of the classes, windows, objects, and functionality of it is extremely reliant on other object, classes, windows, etc.
    Last edited by VirtualAce; 01-02-2006 at 06:53 AM.

  2. #2
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    I see absolutely nothing, though I once spent some time debugging inside the archive code. Perhaps you need to do the same.

    I don't entirely trust your code in Clear(), btw. Wouldn't it clear four times the memory it should? (dwSize times 4 bytes - after the rep, ecx would be 0 and the stosb never executed)
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  3. #3
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Code:
    void Clear(DWORD value=0)
    {
      DWORD *ptrMap=m_pMap;
      DWORD dwSize=m_dwSize;
    
      _asm {
        mov   edi,[ptrMap]
        mov   ecx,[dwSize]
        mov   eax,[value]
        rep   stosd
        and   ecx,03h
        rep   stosb
      }
    }
    This code will clear a DWORD array of any size. If I don't use the and ecx,03h then on odd sized arrays or non-padded arrays (don't line up on right boundaries) it is possible to leave some bytes hanging.

    The array type is DWORD *Array. However just clearing width*height does not clear the entire array. One DWORD is 4 bytes so the total size of a DWORD Array[width*height] is not width*height, but width*height*sizeof(DWORD).

    It seems from a hex viewer that the number of layers being written to the disk is correct. So it seems the problem is prior to the loading code executes, which is where the maps are loaded. So it seems saving is ok.

    Here is the Serialization code from CMapSystem.h
    Note that CMapManager and CMap are not derived from CObject, but I can still use the CArchive::Write() and CArchive::Read() functions on any object regardless of whether or not it is derived from CObject.

    Code:
    void DoArchive(CArchive &ar)
        {
          if (ar.IsStoring())
          {
            MapHeader hdr;
            hdr.dwHeight=m_dwHeight;
            hdr.dwWidth=m_dwWidth;
            hdr.dwSizeBytes=m_dwSize*sizeof(DWORD);
    
            //CString text;
            //text.Format("%u %u %u",hdr.dwHeight,hdr.dwWidth,hdr.dwSizeBytes);
            //::MessageBox(0,text,0,0);
    
            ar.Write(&hdr,sizeof(MapHeader));
            ar.Write(m_pMap,hdr.dwSizeBytes);
    
          }
          else
          {
            MapHeader hdr;
            ar.Read(&hdr,sizeof(MapHeader));
            
            //CString text;
            //text.Format("%u %u %u",hdr.dwHeight,hdr.dwWidth,hdr.dwSizeBytes);
            //::MessageBox(0,text,0,0);
    
            if (m_pMap) delete [] m_pMap;
          
            m_pMap=new DWORD[hdr.dwWidth*hdr.dwHeight];
            
    
            if (m_pMap)
            {
              m_dwWidth=hdr.dwWidth;
              m_dwHeight=hdr.dwHeight;
              m_dwSize=m_dwWidth*m_dwHeight;
              Clear(0xFFFFFFFF);
             
            
              ar.Read(m_pMap,hdr.dwSizeBytes);
            } 
                  
          }
        }
    This is the exact reverse of save and since save works, I dunno why load wouldn't as well.

    Here is the MapManager code:
    Code:
    void DoArchiveForAll(CArchive &ar)
    {
       for (DWORD i=0;i<MapLayers.size();i++)
       {
            MapLayers[i]->DoArchive(ar);
       }
     
    }
    Pretty straightforward. Iterate through the vector and have each map save itself. I thought this better than having the map manager being responsible for saving the map data. After all shouldn't a map object be able to load and save itself?

    Please note that my graphical tile maps save perfect for all layers.

    Maybe this is the problem. I'm creating a CArchive object but I never close it. Do I need to even create a CArchive object? Since I'm overloading OnFileSaveAs(), aren't I just doing what the framework call would do?

    Code:
    void CZeldaEditorDoc::OnFileSaveAs() 
    {
    	// TODO: Add your command handler code here
    	CFileDialog *dlg=new CFileDialog(false,
                                       "*.EPJ",
                                       NULL,
                                       OFN_OVERWRITEPROMPT | OFN_CREATEPROMPT,
                                       "Editor project files (.epj)|*.epj|");
                                       
    
      int result=dlg->DoModal();
    
      if (result==IDOK)
      {
        CFile file(dlg->GetPathName(), CFile::modeCreate | CFile::modeWrite);
        CArchive ar(&file,CArchive::store);
        CMainFrame *ptrFrame=(CMainFrame *)AfxGetMainWnd();
    
        ptrFrame->Serialize(ar);
        ptrFrame->SetWindowText(dlg->GetFileTitle()+".EPJ");
      }
      
    
      delete dlg;
      
    }
    Last edited by VirtualAce; 01-02-2006 at 08:23 AM.

  4. #4
    Registered User
    Join Date
    Jan 2005
    Posts
    847
    This code will clear a DWORD array of any size. If I don't use the and ecx,03h then on odd sized arrays or non-padded arrays (don't line up on right boundaries) it is possible to leave some bytes hanging.
    The number of bytes in your array will always be a multiple of 4 as each element is 4 bytes.

  5. #5
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Exactly. Which is why I'll remove the crappy excess code.



    My bad.

    I'm used to writing a function to clear all types of memory.

    Ok, I fixed the problem and omg I am so retarded. I'm so embarassed to say what it was but here goes. I get the second moron of the new year award.

    I stuck a message box in prior to loading and saving which displayed horizontal size, vertical size, and total size in bytes. Well I added 9 layers and the dialog popped up 10 times (1 for the movement layer which you cannot draw to). Ok good enough. Then I loaded it and it only popped up 2 times. Uh oh. Problem.

    The reason it popped up 2 times is because I'm initializing the map system with 2 default maps when I reset the system in response to a open or close. Now when I save the maps I'm using vector::size() to get the number of maps. See the problem? I can't use vector::size() when I'm adding maps to the vector, because I don't know how many to add.
    So it was doing just what I told it to. There were 2 maps in the vector and thats all it loaded. Well since I'm writing the map header and then the map data guess where that places the file pointer? Right at the width data member of the next map which happens to be a DWORD. So it spits out 32, 60, or whatever the width of the next map is. Doh.

    Really dumb. I'm so sorry to have even stuck this on here. Been up all night coding and I guess this is what I get for it. But damn sometimes the values your code spits out is ...um...totally NOT what you expected to see. The human brain sucks. How can it be so dumbfounded when just minutes earlier it coded the very thing that caused the problem?

    I want a new brain.
    Last edited by VirtualAce; 01-02-2006 at 08:32 AM.

  6. #6
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    OK, I didn't get that dwSize was the number of DWORDs, I thought it was the number of bytes. But at least I got the thing about the second part right.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  7. #7
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Perhaps my variable naming could use some help. dwSizeBytes is the size of the array in bytes.

    Sometimes it's hard to know what to name these things.

  8. #8
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    No, that's fine. I just saw "memset-like construct, dwSize must be bytes".
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  9. #9
    Registered User VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,607
    Well I stray away from using memset because it only operates on bytes. Now I could be wrong and it's hard to tell from the profiler (MSVC profiler is hideous) but it seems to me writing DWORDs would be faster than writing BYTEs.

  10. #10
    Cat without Hat CornedBee's Avatar
    Join Date
    Apr 2003
    Posts
    8,895
    memset implementations are usually the first and most aggressive optimization target for library writers. VC++ comes with a version that is implemented entirely in assembly. Admittedly, it does have to handle special cases (like unaligned start addresses), but it's still pretty fast. It WILL, for example, transfer as much as possible in DWORD-sized blocks. In addition, it's intrinsic, which is a stronger version of inline - the compiler will substitute the function body where you call it, eliminating call overhead. The compiler might even be smart enough to kick out dead code then (i.e. special cases that can't happen). The __aligned keyword on the pointer might help. (Or perhaps that keyword is a flight of fancy.)

    My point is, I don't think clearing the map in a map editor is performance-critical enough to avoid a call to memset.
    All the buzzt!
    CornedBee

    "There is not now, nor has there ever been, nor will there ever be, any programming language in which it is the least bit difficult to write bad code."
    - Flon's Law

  11. #11
    Registered User
    Join Date
    Jan 2005
    Posts
    847
    Quote Originally Posted by Bubba
    Now I could be wrong and it's hard to tell from the profiler (MSVC profiler is hideous) but it seems to me writing DWORDs would be faster than writing BYTEs.
    Thats right and writing QWORDS on 64 bit processors should be even faster.
    My point is, I don't think clearing the map in a map editor is performance-critical enough to avoid a call to memset.
    It dosen't hurt to optimize allthough optimization is best suited to code which is called many times as the gain from optimizing is multiplied by the amount of interations.

    I try to avoid use of the _asm keyword. The compiler has to save all of the registers and restore them after the block of asm code. I think it's better to compile asm functions with an assembler and link the .obj file into the project.

  12. #12
    Registered User
    Join Date
    Mar 2004
    Posts
    220
    Quantum1024 << The compiler has to save all of the registers and restore them after the block of asm code.

    Does that take alot of time? o_O.
    OS: Windows XP Pro CE
    IDE: VS .NET 2002
    Preferred Language: C++.

  13. #13
    Registered User
    Join Date
    Jan 2005
    Posts
    847
    Quote Originally Posted by Tronic
    Quantum1024 << The compiler has to save all of the registers and restore them after the block of asm code.

    Does that take alot of time? o_O.
    No not realy but it adds up if your calling the function many times.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Beginner Needs help in Dev-C++
    By Korrupt Lawz in forum C++ Programming
    Replies: 20
    Last Post: 09-28-2010, 01:17 AM
  2. how do you resolve this error?
    By -EquinoX- in forum C Programming
    Replies: 32
    Last Post: 11-05-2008, 04:35 PM
  3. Quantum Random Bit Generator
    By shawnt in forum C++ Programming
    Replies: 62
    Last Post: 06-18-2008, 10:17 AM
  4. Making C DLL using MSVC++ 2005
    By chico1st in forum C Programming
    Replies: 26
    Last Post: 05-28-2008, 01:17 PM
  5. DX - CreateDevice - D3DERR_INVALIDCALL
    By Tonto in forum Game Programming
    Replies: 3
    Last Post: 12-01-2006, 07:17 PM