Posted by sand1024 on Apr 13, 2025; 8:23am
URL: https://forum.librecad.org/fixed-thanks-please-a-tip-for-the-RS-Eventhandler-tp5726883p5726915.html

well, yet the implementation may be improved a little:

1) Please note that the common approach for states management of actions - just create an enum that contains each state value and refer that enum values in setStatus()/getStatus() instead of raw int numbers.
This significantly improves readability.

2) the location of src\lib\actions\lc_overlayslideaction.cpp seems not the optimal - the \lib mostly contains base code. The lc_overlayboxaction.h  is there since it's used for the basic entities selection.
Yet that implementation... well, it's basically a subset of RS_PreviewActionInterface.

Basically lc_overlayslideaction may be eliminated at all, if you'll inherit LC_ActionFileViewSlide  from RS_PreviewActionInterface (plus, there are more specific methods - say, you can rely on onMouseRightButtonRelease method for handling right button click).

4) I didn't dig deeply into the implementation, but this code seems to be suspicious for me:


void RS_OverlaySlide::draw(RS_Painter* painter)
{
    LC_GROUP_GUARD("Colors");
    bool darkBackground = true;
    auto bgc = QColor(LC_GET_STR("background", RS_Settings::background));
    const RS_Color &fillBackground{bgc};
    if((bgc.red() * 299 + bgc.green() * 587 + bgc.blue() * 114) / 1000 >= 125)
    {
        darkBackground = false;
    }

    painter->fillRect(0, 0, m_width, m_height, fillBackground);

    slide_draw_qpainter(painter,
                        0,
                        0,
                        m_width,
                        m_height,
                        darkBackground,
                        qUtf8Printable(m_fileName));
}

Am I correct that the slide_draw_qpainter is called on each draw()/repaint invocation? An so each time the  the redraw is needed, the slide is loaded from the file?

Hope this not true, yet still please check.

5) Oh, and just forgot another thing - why you need to draw a slide as overlay? What is the logic of the action - you'd like just to let the user preview the slide?