Skip to content

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

@zopsicle

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.

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