theproadam / renderXF

High performance software rendering in c#
MIT License
11 stars 4 forks source link

"clear" color leak on edges when using BlitIntoBitmap #5

Open kf6kjg opened 3 years ago

kf6kjg commented 3 years ago

This might be a problem in my code, but my testing seems to be ruling that out.

Here's my camera setup:

GL.SetMatrixData(FOV, 128f, 1f); // 0=Perspective, 1=Orthographic

GL.ForceCameraPosition(new renderX2.Vector3(128f, 128f, 4000f));
GL.ForceCameraRotation(new renderX2.Vector3(0f, 180f, 0f)); // Angles degree

I then make a mesh that occupies from 0,0 to 256,256 - the Z value is variable. I've validated that the terrainVertexPoints array that feeds the vertex buffer has that range via debugging.

I then render the mesh using

GL.Clear(255, 0, 0);
GL.ClearDepth();
...
using (var vertexBuffer = new GLBuffer(terrainVertexPoints, terrainVertexStride, MemoryLocation.Heap))
{
    GL.SelectBuffer(vertexBuffer);
    var terrainShader = new Shader(shaders.ScaleCorrectionVS, shaders.TerrainShaderFS, GLRenderMode.Triangle);
    terrainShader.SetOverrideAttributeCount(terrainVertexStride - 3); // UVZ
    GL.SelectShader(terrainShader);
    GL.Draw();
}
...
GL.BlitIntoBitmap(mapbmp.Bitmap, new Point(0, 0), new Rectangle(0, 0, mapbmp.Width, mapbmp.Height)); // 1px clear color on top, left, and bottom.

You can see the 1 pixel red lines on the edges: map-1-2001-1999-objects

To test what was going on:

  1. I changed to GL.SetMatrixData(FOV, 64f, 1f) so that I was certain that the terrain fully filled the viewport, and still got the same red lines.
  2. I changed the clear color, the lines changed to the same color.
  3. I tried adjusting the BlitIntoBitmap Point and Rectangle - this demonstrated the ability to remove the red "clear" color lines in some configurations, but at the cost of introducing black lines. This seems to indicate that there might be an off-by-one error or two in the BlitIntoBitmap method, but is not full proof.
kf6kjg commented 3 years ago

Actually, the "red" line is larger than 1 px, and the offsets on BlitIntoBitmap are simply masking it off. This is deeper something's not putting color on those edges.

kf6kjg commented 3 years ago

That's interesting. I set the FS to

if (faceIndex < 50)
{
    bgr[0] = 0;
    bgr[1] = 255;
    bgr[2] = 0;
    return;
}
bgr[0] = 255;
bgr[1] = 255;
bgr[2] = 255;
return;

That should have forced all the terrain to pure white excepting a green strip on the first 50 faces. However here's what I got instead: map-1-1500-1600-objects Be sure to zoom in and check the edges and corners. Notice the red still exists, and where it should be green the red and the green are merged into a yellow. Also note the color banding that bleeds into the image for almost 20 pixels. Something strange is at work here.

EDIT: I checked the values in the bitmap immediately after drawing. The bleed is a compression artifact and is probably nothing to do with what's going on here: image As you can see from the hexadecimal, the first row of pixels is red, other than a few that are green. After that the next row is white other than the leftmost pixel on each row and the last row.

I also attempted placing a screenspace shader in the pipeline that forced all pixels white. That eliminated the red edges, proving for certain that the red is introduced in the render pass somewhere.

theproadam commented 3 years ago

@kf6kjg Alright, I think I know what might be causing that. Il start working on a fix in the morning.

kf6kjg commented 3 years ago

My research indicates that after the return of the FS call inGeometryPath_Fill::FillFull the DrawingBuffer, accessible via bptr or iptr in that class, is correct. However once the Parallel.For in renderX_Core::Draw that calls the previous is complete, the DrawingBuffer is already messed up. The pointer math all looks correct, though I'm not a fan of i and o and other short cryptic variable names! :P

theproadam commented 3 years ago

@kf6kjg I believe I have fixed the issue with the unfilled lines on the left of the screen. You can see how the x = 0 pixels are now filled:

This fix might result in your attributes being extrapolated so its critical that you clamp them.

I have released a v0.3.3b version with this issue fixed.

As for the the bar on the top and the bottom il try to work out a fix soon, once I'm able to reproduce the problem. EDIT: I have been able to partially reproduce the problem. Working on a fix now.

theproadam commented 3 years ago

Alright I think I have fixed the issue. For the left most pixel issue, I just had to port over the XFDraw solution to the problem, however the top and bottom pixel issue was related to floating point imprecision and I have added Math.Abs(value1 - value1) < delta checks for these inside the scanline interpolator. Note it is still possible to make pixels skip the very top of the image due to integer truncation, and while a fix is ready, I want to make sure I don't break anything on accident by enabling it.

Without the fix, the pixels at the very bottom weren't filled, also this fix should fix the top pixels too.

FYI, there is a very small chance of an exception called throw new Exception("this shoudnt happen"); being triggered. If it does, please report it.

kf6kjg commented 3 years ago

Just gave it a run using the code at commit 1dca3ebec99c02f3cf1f98363be30bc37bf10d76. I still have the red.. :/ map-1-1500-1600-objects

theproadam commented 3 years ago

@kf6kjg Hmm, can you try setting your GL.SetMatrixData(FOV, 128f, 1f); to something like GL.SetMatrixData(FOV, 128.01f, 1f); or GL.SetMatrixData(FOV, 127.99f, 1f); and trying again?

kf6kjg commented 3 years ago

Rendered at 128.01f, has red on all 4 edges: map-1-1500-1600-objects

Rendered at 127.99f, has red on left edge, and some on the bottom. Also gained some other random pixels worth of color somehow. Re-rendering still shows that semi-random diagonal line of bad pixels. map-1-1500-1600-objects

As an extreme test I rendered with GL.SetMatrixData(FOV, 64.0f, 1f), still has red lines: map-1-1500-1600-objects

theproadam commented 3 years ago

@kf6kjg Since I'm currently trying to reproduce the problem with your exact configuration, can you tell me what the resolution of your mesh is? Ie quads per unit (0-256). Also the resolution of the color buffer would be nice.

kf6kjg commented 3 years ago

The mesh is square and occupies the range 0-256 units in both axes. In this range there are two tris for every unit, due to filling the quad defined by the square between the pixels of the heightmap that is 256x256 pixel: each pixel is converted to a vertex.

In fact here's an STL of the one I've been testing, exported right out of my vertex buffer: region.zip

My testing VS and FS are pretty simple:

public unsafe void ScaleCorrectionVS(float* OUT, float* IN, int Index)
{
    OUT[0] = IN[0];
    OUT[1] = IN[1];
    OUT[2] = IN[2];
}

public unsafe void TerrainShaderFS(byte* bgr, float* attributes, int faceIndex)
{
    bgr[0] = 255;
    bgr[1] = 255;
    bgr[2] = 255;
    if (faceIndex < 50)
    {
        bgr[0] = 0;
        bgr[1] = 255;
        bgr[2] = 0;
    }
}

The value _pixelScale is 512, the bitmap I'm rendering into is 512x512px. Thus

var GL = new renderX(_pixelScale, _pixelScale); // 512, 512

//...lots of code...

GL.BlitIntoBitmap(mapbmp.Bitmap, new Point(), new Rectangle(0, 0, mapbmp.Width, mapbmp.Height)); // 512,512
kf6kjg commented 3 years ago

lol, the above review points out to me that I'm putting too many vertices into my mesh: it's 512x512 even though it only has 256x256 data in the base terrain heightmap.

EDIT: Fixing the code to a 256x256 resolution mesh doesn't change the red line. Even dropping it to 64x64 doesn't do anything.

theproadam commented 3 years ago

So based of the information here, I have to assume the issue is related to the rasterization. The first problem relates to how renderXF translates its "Normalized" world coordinates into screen space coordinates:

VERTEX_DATA[im * Stride + 0] = roundf(rw + VERTEX_DATA[im * Stride + 0] / ox);
VERTEX_DATA[im * Stride + 1] = roundf(rh + VERTEX_DATA[im * Stride + 1] / oy);

In the last release roundf was nothing, however since screenspace coordinates cannot have decimal numbers, something has to be done. This could be set to a) nothing, b) truncate to integer or c) rounding.

The second problem relines in the ScanLinePLUS function that doesn't detect at least two intersects in order to draw a scanline. Unfortunately I'm not sure exactly how this would deal with a intersect and I might as well rewrite it.

Regardless I am releasing v0.3.4 which has rounding on the XYZ to XY transforms and loosened epsilons for the bigger or equal checker. Additionally I have improved the performance for tiny triangles by ~3x (at the cost of big triangles) by removing the extra parallelization loop inside for the FillFull() function, and added wireframe support for non nulled vertex shader.

I have tested this update with a 256x256 image and a 256 by 256 mesh and I didn't have any pixels not being filled. Hopefully this fix works, otherwise I will have to rewrite and reinforce the scanline algorithm, which isin't all that bad since I have had fixing unfilled corner pixels on XFDraw's to do list.

kf6kjg commented 3 years ago

So... No change. That said I do very much love the amount of effort you are putting into this. Here's the result of my own digging, though I think I've mentioned it before: image As you can see immediately after the call to Parallel.For(0, SB.FaceCount, ops.FillFull); I can detect invalid color on the first pixel of the first scanline.

If I do a similar setup at the end of the relevant section of FillFull, I detect no error: image Note that neither breakpoint shown in this image ever get hit. This implies to me that the frag shader result is working correctly, but that something between the frag shader and the return of the FillFull method is the issue.

Right now the only thing that seem suspicious, but if it's really a problem it would only explain the vertical line, is the FromX + 1 in

for (int o = FromX + 1; o <= ToX; o++)

I don't pretend to full understand all of what's written in this method, but it feels like this would miss the 0th pixel. My debugging indicates that FromX is never less than 0, so that means that o always starts at 1, and since that's multiplied by sD in the FS call, that implies that the first column never gets a frag shader called upon it. I've tried a few ways to attempt to "fix it", but all have resulted in me breaking an assumption somewhere and crashing.

theproadam commented 3 years ago

Hmmm, int o = FromX + 1 was from 3 versions ago. It seems like I may have messed up the file release. I will release a v0.3.4b and hopefully it should be the correct assembly!

Heres the release v0.3.4b

kf6kjg commented 3 years ago

I pull straight from master. And that commit doesn't look right, you removes a couple dozen files and left an unresolved merge in the tree... :/

theproadam commented 3 years ago

Sorry, I accidentally merged the files incorrectly, and thus I had to delete the incorrectly copied ones. I'm a mech eng student and not a software eng student so I'm not very good with github and git in general.

kf6kjg commented 3 years ago

I was a mech engineer for 15 years designing routers and saws for granite, and a software engineer all through and since. Hopefully I can help. I've created a Pull Request that might help keep the source code tree clean.

I'll try again with the new code.

kf6kjg commented 3 years ago

Ok, that's very different. The red edges are now gone, but in their place is a grid of red dots: map-1-1000-1000-objects Note that the Y axis is flipped on the image since the last time I posted images: I added a new line to correct the upside down output:

GL.BlitIntoBitmap(mapbmp.Bitmap, new Point(), new Rectangle(0, 0, mapbmp.Width, mapbmp.Height));
mapbmp.Bitmap.RotateFlip(RotateFlipType.RotateNoneFlipY); // Fix the image being upside down.

So thus the first scanline is now on the bottom and the grid of dots is in the last few scanlines.

[EDIT: Side note I had to use the changes that are in PR #7 to accomplish this.]

theproadam commented 3 years ago

Hmm can you try adding roundf here inside of the FillFull() method:

...
else if (matrixlerpv == 1)
for (int im = 0; im < BUFFER_SIZE; im++)
{
      VERTEX_DATA[im * Stride + 0] = roundf(rw + VERTEX_DATA[im * Stride + 0] * iox);
      VERTEX_DATA[im * Stride + 1] = roundf(rh + VERTEX_DATA[im * Stride + 1] * ioy);

      if (VERTEX_DATA[im * Stride + 1] > yMaxValue) yMaxValue = VERTEX_DATA[im * Stride + 1];
      if (VERTEX_DATA[im * Stride + 1] < yMinValue) yMinValue = VERTEX_DATA[im * Stride + 1];
}

Additionally you can try tightening the const float SCANLINE_EPSILON = 1E-2f; to something like 1E-3f

kf6kjg commented 3 years ago
  1. Adding the roundf without the epsilon change reduced the grid to a single row of even-odd pixels on the last scanline.
  2. Changing epsilon to 1E-3f without roundf reduced the grid to two rows row of even-odd pixels on the last 3 scanlines.
  3. Changing epsilon to 1E-5f without roundf was only a single row on the last scanline, with some gaps.
  4. Changing epsilon to 1E-3f with roundf was the same as having roundf on its own.
  5. Changing epsilon to 1E-5f with roundf was the same as having roundf on its own.

Here's the result of test 5 above: map-1-1000-1000-objects Note again that the topmost row of pixels is the last scanline.

That said, it seems that the attributes passed to the frag shader are borken: all three values are NaN.

theproadam commented 3 years ago

Hmm, I don't recall changing the attribute code, however il go check if see if I broke it. In the meanwhile can you try this small last change, as it will tell me if the scanline algorithm or the XYZ to XY transforms are at fault here:

In FillFull() find

yMax = (int)yMaxValue;

Replace this with

yMax = (int)yMaxValue + 1;

OR, this:

yMax = (int)yMaxValue == renderHeight - 2 ? renderHeight - 1 : (int)yMax;
kf6kjg commented 3 years ago

Ok, latest combinations: Baseline is as before: no roundf, no epsilon change, no ymax change. Has the 3 rows of red dots on the last 5 scanlines. map-1-1000-1000-objects

With the yMax = (int)yMaxValue + 1; change it reduced to a single row of red dots on the last scanline. map-1-1000-1000-objects

With the complex yMax evaluation change, it is almost all red. map-1-1000-1000-objects

EDIT: Combining with either the roundf or the epsilon, or both, changes doesn't produce a better outcome.

theproadam commented 3 years ago

Alright, it seems like my only remaining choice is to reinforce the ScanLinePLUS() method. I should be able to fix it about a day or two.

theproadam commented 3 years ago

Okay, sorry for the delay, I got a bit carried away writing XFDraw's new shader parser/compiler, but regardless I think I've fixed the problem. Can you test the fix by replacing the ScanLinePlus() method with this one:

unsafe bool ScanLinePLUS(int Line, float* TRIS_DATA, int TRIS_SIZE, float* Intersects)
        {
            int IC = 0;
            for (int i = 0; i < TRIS_SIZE - 1; i++)
            {
                float y1 = TRIS_DATA[i * Stride + 1];
                float y2 = TRIS_DATA[(i + 1) * Stride + 1];

                if (y2 == y1 && Line == y2){
                    LIPA_PLUS(Intersects, 0, TRIS_DATA, i, i + 1, Line);
                    LIPA_PLUS(Intersects, 1, TRIS_DATA, i + 1, i, Line);
                    return true;
                }

                if (y2 < y1){
                    float t = y2;
                    y2 = y1;
                    y1 = t;
                }

                if (Line <= y2 && Line > y1){
                    LIPA_PLUS(Intersects, IC, TRIS_DATA, i, i + 1, Line);
                    IC++;
                }

                if (IC >= 2) return true;
            }

            if (IC < 2)
            {
                float y1 = TRIS_DATA[0 * Stride + 1];
                float y2 = TRIS_DATA[(TRIS_SIZE - 1) * Stride + 1];

                if (y2 == y1 && Line == y2){
                    LIPA_PLUS(Intersects, 0, TRIS_DATA, 0, (TRIS_SIZE - 1), Line);
                    LIPA_PLUS(Intersects, 1, TRIS_DATA, (TRIS_SIZE - 1), 0, Line);
                    return true;
                }

                if (y2 < y1){
                    float t = y2;
                    y2 = y1;
                    y1 = t;
                }

                if (Line <= y2 && Line > y1){
                    LIPA_PLUS(Intersects, IC, TRIS_DATA, 0, TRIS_SIZE - 1, Line);
                    IC++;
                }
            }

            if (IC == 2) return true;
            else return false;
        }

and then replacing these lines right after ScanLine() is called:

... 
{
    FROM = Intersects;
    TO = Intersects + (Stride - 1);
}

FROM[0] = roundf(FROM[0]);
TO[0] = roundf(TO[0]);

FromX = (int)FROM[0] == 0 ? 0 : (int)FROM[0] + 1;
ToX = (int)TO[0];

slopeZ = (FROM[1] - TO[1]) / (FROM[0] - TO[0]);
bZ = -slopeZ * FROM[0] + FROM[1];

if (ToX >= renderWidth) ToX = renderWidth - 1;
if (FromX < 0) FromX = 0;

float ZDIFF = 1f / FROM[1] - 1f / TO[1];
...

Lastly, ensure the roundf is still there and increase the epsilon to a very tiny number as there is no need for it anymore.

As for updating to newer net standards, I plan on rewriting renderXF with XFDraw's code once its done, but with the only difference being that the rendering would still be done in c#. This would allow for nice things such as being able to have multiple out's in the screenspace or fragment shaders.

kf6kjg commented 3 years ago

Combining all that together on top of latest master it's perfect: all red lines and dots are gone.

Good news on XFDraw. I look forward to integrating it when it's ready: the way I built Anaximander2 it can have multiple renderers that are user selectable.

kf6kjg commented 3 years ago

I built a variant of the texturetest package that demonstrates the clear color leakage: https://github.com/kf6kjg/renderXF/tree/anax_issues_demo

The above changes seem to fix it, so if those could land in the repo it'd be great.