What is wrong with this function!!!

This is a discussion on What is wrong with this function!!! within the C Programming forums, part of the General Programming Boards category; Hello, im working on a graphics library for dos and my getpixel function crashes at certain points on the screen. ...

  1. #1
    Registered User
    Join Date
    Sep 2001
    Posts
    140

    What is wrong with this function!!!

    Hello, im working on a graphics library for dos and my getpixel function crashes at certain points on the screen. It doesnt crash in standard vga mode (320x200x256) but it does in svga vesa modes, My best bet is it has something to do with bank switching.

    Code:
    //reads the pixel at xy, value is which one
    //in in 256 color mode value doesnt matter but
    //in 16m color mode value determines what value to grab
    //1 is red 2 is green 3 is blue
    int getpixel(unsigned int x, int y, int value)
    {
       long offset=0L;
       int val;
    
       if(x>SCREENWIDTH) // if its off the screen
       return 0;
       if(y>SCREENHEIGHT) // if its off the screen
       return 0;
       if(y<0)
       return 0;
    
    
       if(GFXMODE==0) // if its standard vga mode (no bank switching)
       {
            x=((y*SCREENWIDTH)+x);
            asm{
                    mov bx, 0A000H
                    mov es, bx
                    mov bx, x
                    mov ax, [es:bx]
    	mov val, ax
            }
            return val;
       }
       
       if(BPP==1) // if its a 256 color mode (1 byte per pixel)
       {
            offset=(long)y*SCREENWIDTH+(long)x; //get the offset
            OldBank=bank; //self explanatory 
            bank=offset>>16; // sets the bank
            x=offset; // x is now the offset for the current bank
    
            if(OldBank!=bank) // if it needs to bank switch
            {
            asm{
                    // set read bank
                    mov ax, 04F05H
                    mov bx, 0
                    mov dx, bank
                    int 10H
    
                    // set write bank
                    mov ax, 04F05H
                    mov bx, 1
                    mov dx, bank
                    int 10H
    
            }
            }
            // now read the pixel
            asm{ 
                    mov bx, 0A000H 
                    mov es, bx
                    mov bx, x
                    mov ax, [es:bx]
                    mov val, ax
            }
       	return val;
       }
    
       // this is pretty much the same as 1 BPP except it reads 3 bytes instead of 1, its 16.7m color mode
       if(BPP==3)
       {
            offset=(long)y*SCREENWIDTH+(long)x;
            offset*=3;
            OldBank=bank;
            bank=offset>>16;
            x=offset;
    
            if(OldBank!=bank)
            {
            asm{
                    // set read bank
                    mov ax, 04F05H
                    mov bx, 0
                    mov dx, bank
                    int 10H
    
                    // set write bank
                    mov ax, 04F05H
                    mov bx, 1
                    mov dx, bank
                    int 10H
    
            }
            }
    
            asm{
                    mov bx, 0A000H
                    mov es, bx
                    mov bx, x
            }
    
            if(value==3) // if they wanna read blue
            {
            	asm{
            	        mov ax, [es:bx]
            	        mov val, ax
            	}
            	return val;
            }
            if(value==2) // or green
            {
            	asm{
            	        inc bx
            	        mov ax, [es:bx]
            	        mov val, ax
            	}
            	return val;
            }
            if(value==1) // or red
            {
            	asm{
            	        inc bx
            	        inc bx
            	        mov ax, [es:bx]
            	        mov val, ax
            	}
            	return val;
            }
    
       }
    
    	return 0;
    }
    thanks for any help

  2. #2
    Registered User
    Join Date
    Sep 2001
    Posts
    140
    Are you people that stupid, not one out of like a few hundred users can answer this. This is why cprogramming.com sucks, it used to be cool.

  3. #3
    Mayor of Awesometown Govtcheez's Avatar
    Join Date
    Aug 2001
    Location
    MI
    Posts
    8,825
    What kind of points does it crash on?

  4. #4
    Registered User
    Join Date
    Sep 2001
    Posts
    140
    Im not sure exactly but my putpixel also draws the wrong color at the same places, its probably either the end or beginning of a bank because it doesnt happen in vga mode.

    Its gonna be pretty sad if someone at flashdaddee answers this first considering how few people are over there.

  5. #5
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,417
    > Are you people that stupid, not one out of like a few hundred users can answer this
    Perhaps if you'd waited more than 6 hours for a reply, you would attract more sympathy.

    And since this is a C board, not an ASM hacker board with C wrappers, the vast majority of newbies wont be able to help.

    Most people only visit once a day, and you should therefore expect to have to wait up to 24 hours for perhaps the right person to answer the question (assuming they exist). The world doesn't begin at EST and end at PST you know.

    > offset*=3;
    Given that multiples of 3 are not even divisors of 65536, the occasional pixel will straddle two banks in the BPP==3 mode.

    I'm taking you at your word that you know banks are 65536 in length, and not say 64000, because both are in some sense 64K.

    Range checking x and y wouldn't go amiss - while you're debugging.

    Nor would some test code which deliberately loops over all possible x and y values. Then you would be able to at least answer the question - which specific X and Y values cause the problem. Knowing that x=212 and y=478 say would at least help....

  6. #6
    Registered User
    Join Date
    Sep 2001
    Posts
    140
    The x and y values it crashes at are different for each video mode so it would be poinless to give you those. Im gonna go run up to the puter lab and see if I can find out where it crashes.

  7. #7
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    > The x and y values it crashes at are different for each video mode

    Really? You think so? Wow, who'd have guessed? This is like saying "My screen area changes in each screen mode!" It's a given.

    > so it would be poinless to give you those.

    Well, apparently then you have it all figured out. What do you need us for again?

    > Im gonna go run up to the puter lab and see if I can find out
    > where it crashes.

    Yeah, debugging is generally a good idea.

    Quzah.
    Hope is the first step on the road to disappointment.

  8. #8
    Super Moderator VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,590
    Failing to preserve dx across int 10h call
    One serious problem that you have is that you are not pushing the value of dx prior to executing int 10h. If int 10h changes the value of dx then when you set currentbank to dx, it is not what you expect it to be. This in itself is incurring that on every pixel your program is bankswitching, even when it does not need to. Make sure you preserve dx across the int 10h call.

    Don't check for screen boundaries
    Also do not check for the screen boundaries. These checks will slow the entire system down since it will check this for every pixel. Check for this in another functions such as a clipping function. If you are only doing 2D graphics then you probably have no choice but to do those checks both on GetPixel. PutPixel need not check boundaries. Assume that the programmer knows that he or she should not try to get the color of a pixel that does not exist. Similar to C's error checking. It is the programmer's responsibility to be aware of what he or she is doing. This will save a lot of cycles since you do not have to babysit for the programmer.

    Problem with 24-bit modes
    Salem is right in that in 24 bit mode their will be a time in which half a pixel lies in the next bank. This only happens in 24 bit modes. Very annoying.

    This is for 16-bit modes only, 24-bit was way too slow in DOS and not worth the hassle of the bank problem.

    This can be altered to GetPixel with a few minor changes.
    Code:
    void PlotPixel(long poffset,WORD color)
    {
       //16 bit color offset
       register int aoffset=poffset & 0xFFFF;
       register int curbank=poffset>>16;
       asm {
         START:
           mov ax,[curbank]
           cmp ax,[bank]
           jz PLOTPIXEL
         SWITCHBANKS:
           mov ax,4f05h
           mov bx,0
           mov dx,curbank
           push dx             //Preserve dx across int 10h
           int 10h
           pop dx  	//Get previous dx off the stack
         UPDATEBANK:
           mov [bank],dx
         PLOTPIXEL:
           mov ax,0a000h
           mov es,ax
           mov di,WORD [aoffset]
           mov ax,color
           stosw
       }
    }

    It would be even faster to do a STOSD instead of STOSW but Borland does not allow that instruction. You absolutely must push dx prior to int 10h.

    Code two separate functions for VESA and non-VESA
    I would code two different functions for VESA and non-VESA modes. Use function overrides so that the compiler chooses which one it will use - that way you do not have to waste cycles checking which one to use.

    void PlotPixel(long offset,BYTE color);
    void PlotPixel(long offset,WORD color);

    WORD GetPixel(long offset);
    BYTE GetPixel(long offset);

    You are checking to see if you are in VESA mode on every pixel. Once you set the mode, you know you are - so to check on each pixel is a waste of precious cycles.

    Compute offset elsewhere or do not use x and y coords
    Also, do not, if you can help it, compute the pixel offset within the PlotPixel/GetPixel function. You could #define the conversion function or better yet you could set up your graphics to use continuing offsets into the buffer instead of x and y coords. This will take some careful planning and effort on your part, but it will be well worth it.

    You can alter my above PlotPixel function to do the reverse op or GetPixel still using stosw - though I've not converted it yet. Also your checks for the RGB stuff will slow the system down. Simply have your function retrieve the word. Then create a function that will take a WORD and return RGB in a struct. You want your pixel retrieval function to just retrieve the pixel - extract the RGB's later.

    Here is a slower example of a getpixel function w/o using stosw and converting from word to RGB.
    This assumes 565 RGB.
    Code:
    RGB GetPixel(DWORD offset)
    {
      register int aoffset=offset & 0xFFFF;
      register int curbank=offset>>16;
      WORD color=0;
      asm {
       START:
         mov ax,[curbank]
         cmp ax,[bank]
         jz GETPIXEL
       SWITCHBANKS:
         mov ax,4f05h
         mov bx,0
         mov dx,curbank
         push dx
         int 10h
         pop dx	//Get previous dx
      UPDATEBANK:
         mov [bank],dx
      GETPIXEL:
        mov ax,0a000h
        mov es,ax
        mov bx,WORD [aoffset]
        mov ax,es:[bx]
        mov [color],ax
      }
      //Should be elsewhere
      RGB temp;
      temp.grn=(color &0x7e0)>>6;
      temp.red=(color &0xF800)>>11;
      temp.blu=(color & 0x1f);
      return temp;
    }
    This does work on my machine and it does not crash.
    Hope this helps. Not sure why yours is crashing. Just gave some tips on how to speed it up.

    For PlotPixel you can get some speed gains by doing this.
    Code:
    //These are global or in a struct - your choice
    BYTE REDS[32];
    BYTE BLUS[32];
    BYTE GRNS[32];
    
    for (int i=0;i<32;i++)
    {
       REDS[i]=i<<11;
       GRNS[i]=i<<6;
       BLUS[i]=i;
    }
    
    #define RGBTOWORD (r,g,b) (WORD)((REDS[r])+(GRNS[g])+(BLUS[b]))
    
    WORD color=RGBTOWORD(red,grn,blu);


    Your assembly looks pretty tight, though. Let me know when you are ready to chuck this bank switching stuff and move to protected mode.

    Questions? PM me and I'll be glad to help. Is this for your new library? Looks good.

  9. #9
    Super Moderator VirtualAce's Avatar
    Join Date
    Aug 2001
    Posts
    9,590
    GetPixel need not check for boundaries. Sorry, typo.

  10. #10
    Registered User
    Join Date
    Sep 2001
    Posts
    140
    Thanks for all the good feedback, Im actually just learning assembly and Im not sure of all of the instructions, Ill look up stosw when I get home today. And ya this is for my new library.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. In over my head
    By Shelnutt2 in forum C Programming
    Replies: 1
    Last Post: 07-08-2008, 06:54 PM
  2. Game Pointer Trouble?
    By Drahcir in forum C Programming
    Replies: 8
    Last Post: 02-04-2006, 01:53 AM
  3. Replies: 3
    Last Post: 03-04-2005, 01:46 PM
  4. structure vs class
    By sana in forum C++ Programming
    Replies: 13
    Last Post: 12-02-2002, 06:18 AM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21