Skip to content

Confusing mistake in Brush::ClipToBrush pseudocode in MAPFiles.pdf #1

Description

@zopsicle

First of all, thanks for this insightful document! It has been very helpful while implementing a MAP compiler.

In the document MAP files file format description, algorithms, and code, the pseudocode implementation of Brush::ClipToBrush is:

void Brush::ClipToBrush ( Brush *pBrush, bool bClipOnPlane )
{
    for i = 0 to NumberOfPolygons – 1
    {
        Polygons[ i ] = Polygons[ i ].ClipToList ( pBrush.Polygons, bClipOnPlane );
    }
}

But I think it should be:

void Brush::ClipToBrush ( Brush *pBrush, bool bClipOnPlane )
{
    for i = 0 to NumberOfPolygons – 1
    {
        Polygons[ i ] = pBrush.Polygons.ClipToList ( Polygons[ i ], bClipOnPlane );
    }
}

I had a hard time understanding the implementation of ClipToList since it was treating this (Polygons[ i ]) as a list of polygons, whereas it is actually a single polygon. After some googling I found this repository and within it the actual implementation:

map-files/brush.cpp

Lines 8 to 31 in f28e6a3

void Brush::ClipToBrush ( Brush *pBrush_, bool bClipOnPlane_ )
{
Poly *pPolyList = NULL;
Poly *pPoly = m_pPolys;
for ( int i = 0; i < GetNumberOfPolys ( ); i++ )
{
Poly *pClippedPoly = pBrush_->GetPolys ( )->ClipToList ( pPoly, bClipOnPlane_ );
if ( pPolyList == NULL )
{
pPolyList = pClippedPoly;
}
else
{
pPolyList->AddPoly ( pClippedPoly );
}
pPoly = pPoly->GetNext ( );
}
delete m_pPolys;
m_pPolys = pPolyList;
}

which seems to confirm that the this argument and the first argument to ClipToList are indeed swapped in the document's pseudocode implementation of Brush::ClipToBrush.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions