JamesBremner / DXF_Viewer

A simple DXF File viewer
MIT License
22 stars 4 forks source link

Bounding box of arc is the entire circle #15

Closed asmwarrior closed 4 years ago

asmwarrior commented 4 years ago

Hi, I see such code:

void CArc::Update( cBoundingRectangle& rect )
{
    rect.Update( x, y+r );
    rect.Update( x, y-r );
    rect.Update( x+r, y );
    rect.Update( x-r, y );
}

I see that if the arc is a very tiny piece of a circle, the bounding box of the circle will be used.

Something like such code need to be used: math - formula to calculate bounding coordinates of an arc in space - Stack Overflow

JamesBremner commented 4 years ago

Using the bounding box of the complete circle seems reasonable to me. The bounding box is an upper limit to the extent of the drawing, so that the entire drawing will be visible when first opened. Thereafter, user can zoom in to see details.

xubury commented 4 years ago

@JamesBremner using the bounding box of a entire circle will cause some problems. For example, in the screenshots below, if you consider the entire circle, the bounding rectangle of the entire dxf will exceed from the actual bound thanks to the arc at the bottom. QQ图片20200310155507 QQ图片20200310155325 Here's our possible fix:

void CArc::Update( cBoundingRectangle& rect )
{
    uint8_t start_quadrant = (int)sa % 360 / 90 + 1;
    uint8_t end_quadrant = (int)ea % 360 / 90 + 1;

    if(end_quadrant < start_quadrant || (end_quadrant == start_quadrant && (int)sa % 360 >(int)ea % 360 ))
    {
        end_quadrant += 4;
    }
    for(int i = 0; i < end_quadrant - start_quadrant; i ++)
    {
        int j = start_quadrant + i;
        if(j > 4)
        {
            j %= 4;
        }
        switch(j)
        {
            case 1:
                rect.Update(x, y + r);
                break;
            case 2:
                rect.Update(x - r, y);
                break;
            case 3:
                rect.Update(x, y - r);
                break;
            case 4:
                rect.Update(x + r, y);
                break;
        }
    }
    rect.Update( x + r * cos(sa * M_PI / 180), y + r * sin(sa * M_PI / 180) );
    rect.Update( x + r * cos(ea * M_PI / 180), y + r * sin(ea * M_PI / 180) );
}
JamesBremner commented 4 years ago

Some suggestions for your code:

  1. There are no comments! You should add comments that describe what is going on.

  2. Please use variable names that indicate their purpose. Never use variable names such as i, j, etc.

  3. Have you tested this code? A unit test would be good.

Here is the procedure for making contributions to open source code on Github

xubury commented 4 years ago

@JamesBremner Hi, sorry for the uncommented code. I will post the codes with comments soon. I tested the code on 5 different arcs with start anlge at first or second quadrant. Here're the examples I tested with second quadrant start angle, blue points indicate the four corner of the bounding rectangle. QQ截图20200310214906 QQ截图20200310214915 QQ截图20200310214928 QQ截图20200310214936 QQ截图20200310214945

JamesBremner commented 4 years ago

Please test and close this issue if OK.

asmwarrior commented 4 years ago

I think this issue can be closed, thanks everyone.