All my code seems to be wrong

Started by
5 comments, last by Sand_Hawk 21 years, 7 months ago
I'm working on some linedrawer. I finally got it to work as it should be, however, when I plot the graphics something awefully messes up. I have only tested it on 16 bits but that fails. Here is my code for drawing a pixel on the screen:
    
UCHAR *Buffer[(X + Y * (lPitch >> 1)] = RGB16BIT(Red, Green, Blue);
  

With this line I get to see 2 lines and my screen seems to be WAAAY bigger then 640*480. The line is also interlaced. What can it be? My 16bit macro also looks like this:

      
#define RGB16BIT(R, G, B) ((B % 32) + ((G % 32) << 6) + ((R % 32) << 11))
    
It's not the linedrawer, I ran it through the debugger and it seems to work fine(The X, Y values are as they suppose to be) Sand Hawk [EDIT] Changed some code [/EDIT] ---------------- -Earth is 98% full. Please delete anybody you can.
My Site [edited by - sand_hawk on September 20, 2002 4:01:01 PM]
----------------(Inspired by Pouya)
Advertisement
Try taking out the right shift so you have

Buffer[X + Y * lPitch] = RGB16BIT(Red, Green, Blue);

That should work fine

[edited by - Monder on September 20, 2002 2:35:05 PM]
The pitch stuff varies depending on how the pitch is measured (usually in bytes, but not necessarily) and what type your buffer is (usually a 16-bit type such as ''short'', but not necessarily). If it is a short, then I would have thought your code would work. If it is in bytes/chars, then you wouldn''t want to shift it.

The RGB16BIT macro is also a bit suspect too... try replacing the ''% 32'' with ''/ 8'' or maybe ''>> 3''.

[ MSVC Fixes | STL | SDL | Game AI | Sockets | C++ Faq Lite | Boost | Asking Questions | Organising code files | My stuff ]
Well, I removed the >> 1 from the pixel plotter. The result is odd, namely an line wich runs from 10, 10 to ??, ??(Should be 100, 100). The line is dotted wich 2 colors. White, purple, void, white, etc. This is a perfect diagonal line and in code it does every step X++ and Y++. I haven''t changed the RGB macro but it''s ok because the result is the same when I give 255 to the buffer. What else can I do? Here is the line drawer in case someone wants to see it. (Maybe indenting is fubarred after pasting)


  void inline DirectDraw::DrawLine16(int StartX, int StartY, int EndX, int EndY, UCHAR Red, UCHAR Green, UCHAR Blue, UCHAR *Buffer){    int DifferX = 0;    int DifferY = 0;    int DeltaY  = 0;    int XInc    = 0;    int YInc    = 0;    int X       = 0;    int Y       = 0;    int Balance = 0;    DifferX = EndX - StartX; // Bereken het verschil tussen X1 en X0    DifferY = EndY - StartY; // Bereken het verschil tussen Y1 en Y0    DeltaY = DifferY / DifferX;    if (DifferX >= 0) // Lijn van Links naar Rechts        XInc = 1;    else // Rechts naar links        XInc = -1;    if (DifferY >= 0) // Lijn van boven naar onder        YInc = 1;    else        YInc = -1;    // Zet de start posities van X en Y    Y = StartY;    X = StartX;    if (DifferX >= DifferY)    {		DifferY <<= 1;		Balance = DifferY - DifferX;		DifferX <<= 1;        while (X != EndX)		{            PlotPixel16(X, Y, Red, Green, Blue, Buffer);            if (Balance >= 0)			{				Y += YInc;				Balance -= DifferX;			}			Balance += DifferY;			X += XInc;		}        PlotPixel16(X, Y, Red, Green, Blue, Buffer);    }    else    {		DifferX <<= 1;		Balance = DifferX - DifferY;		DifferY <<= 1;		while (Y != EndY)		{			PlotPixel16(X, Y, Red, Green, Blue, Buffer);            			if (Balance >= 0)			{				X += XInc;				Balance -= DifferY;			}			Balance += DifferX;			Y += YInc;		}         PlotPixel16(X, Y, Red, Green, Blue, Buffer);    }}  


I sure hope anyone can help me now.

Sand Hawk
----------------(Inspired by Pouya)
1.

  Buffer[(X + Y * (lPitch >> 1)] = RGB16BIT(Red, Green, Blue);  


Shouldn't you shift left, because lPitch is the width of the image and 16-bit = 2 bytes per pixel? (prefer *2 by the way, compiler can optimize it to bit shifting)

2.
Don't inline the DrawLine-function. It's just too huge. I once tried inlining a largish function and it led to *reduced* runtime speed.

3.
Don't use RGB macro. Use inline function

4.
In RGB function, /8 instead of %32, like Kylotan said. That way it's linear. Now R=G=B=224 will result in a full black pixel.

5.
In DrawLine, don't initialize or even define variables before you use them. Initializing them all with 0 and then soon replacing that with some other number looks goofy. And DeltaY is never used.

Also, your comments are useless.. Don't put comments like "here we assign start position to X and Y". Or "here we calculate the difference between x1 and x0". Those are obvious from the code. You should comment about the algorithm used instead.

  void DirectDraw::DrawLine16(int StartX, int StartY, int EndX, int EndY, UCHAR Red, UCHAR Green, UCHAR Blue, UCHAR *Buffer){    int DifferX = EndX - StartX;    int DifferY = EndY - StartY;    int XInc = (DifferX >= 0) ? 1 : -1;    int YInc = (DifferY >= 0) ? 1 : -1;    int X = StartX; //X and Y are useless variables. You could    int Y = StartY; //use StartX and StartY directly instead    if (DifferX >= DifferY)    {        DifferY /= 2;        int Balance = DifferY - DifferX;//and so on...        
I know only 1. addressed your question ....

[edited by - civguy on September 20, 2002 5:40:05 PM]
Okies, updated the macro but haven't replaced it with inline function yet. However, my PlotPixel(Also updated) still fubars out on me. Here is how it looks like now:


    Buffer[X + Y * (lPitch / 2)] = RGB16BIT(Red, Green, Blue);    


What is going wrong?

Sand Hawk
[EDIT]
Fixed a little more code
[/EDIT]

----------------
-Earth is 98% full. Please delete anybody you can.


My Site

[edited by - sand_hawk on September 20, 2002 6:15:12 PM]
----------------(Inspired by Pouya)

      Buffer[X + Y * (lPitch * 2)] = RGB16BIT(Red, Green, Blue);    

of course.. X needs to be multiplied by 2 too... Otherwise it will just change the color of the same pixel when it should be accessing the next pixel.

      Buffer[(X + Y * lPitch) * 2)] = RGB16BIT(Red, Green, Blue);    

Or alternatively (depending on what lPitch really is), try this:

      Buffer[X * 2 + Y * lPitch] = RGB16BIT(Red, Green, Blue);    

And that still won't work perfectly. Because now it only changes the first byte and not the second one. I think you need to cast it to a unsigned short-pointer first.. Why can't you just have unsigned short array instead of unsigned byte array?

[edited by - civguy on September 20, 2002 6:22:11 PM]

This topic is closed to new replies.

Advertisement