[fixed - thanks] please a tip for the RS_Eventhandler

do you want to use the actual graphic instead of the preview?

You can just add to the container ( RS_GraphicView::getGraphic())

emanuel wrote

The preview is not so good.
But is there another container where I can add that?
Where I don't have to redraw all the time.

I'll try.

well, preview will be redrawn indeed. But why can't you
1) just load the content of the slide after closing the dialog (one time),
2) then save the data needed for slide drawing say, in your action
3) just redraw that saved data when needed instead of parsing the file each time?

Again, I'm not sure I understand the intended purpose for the action - is it just a one-time preview of slide? Or you'd like to insert the slide into drawing (like image)?
So it's hard for me to be more specific, sorrry

emanuel wrote

5) Slide is the Acad standard img format to set on DCL Buttons and Images. The "vslide" is only for displaying it on screen. The format can not be implemented in a dxf or dwg file. AutoCAD uses it for example to preview Blocks in Dialogs, before inserting.

Sadly the command "mslde" is a lot of work more. It creates a slide from the drawing. only from lines and filled polygons.

Well, the Slide format is of course is Autocad-specific thing, yet support of it will be helpful, of course.
But proper support of it is wider than just implementation of specific command...

For example, it may be also supported by Library Widget and Block widget (in addition to png thumbnail), and potentially for file open dialogs.

As for the generation of slide files - well, no sure it's necessary so far, yet potentially it may be useful too.

So from the generic design point if view, it is practical to add

1) The class for generic parsing of slide format (somewhere in /lib)
2) some entity (like RS_Image or RS_EntityContainer) that will be non-persistent in DXF but be drawable (like LC_RefLine)
3) Script command and action will just rely on 1 and 2 above.

lib is here:
https://github.com/emanuel4you/LibreCAD/tree/developer/libraries/libslide

Widget (planed as extra file):
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/lib/scripting/lisp/Types.h#L878

console slides to slidlib:
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/main/console_slidelib.cpp

command:
I need to get to know the command structure better.
Then it'll be fine. Right now, it's a lot of trial and error.

Oh, yes, that's exactly what I've mentioned... great.

As for action you'd like to use - you can investigate RS_ActionDrawImage as startging point.
Of course, there is lots of image specific code for parameters (like angle etc).)

But this action could be a good start - least, you could see which methods are overriden, state management etc.

Actually, other actions present in /src/actions/drawing and so on could be a good source of information. However, RS_ActionDrawImage  is quite a minimalistic for the start...

Ok :-)

I have now set it up as described as an action with its own entity:

action:
void LC_ActionFileViewSlide::drawOverlaySlide(const QString &file)
{
    slide = new LC_Slide(graphic, RS_Vector(graphicView->getWidth(), graphicView->getHeight()), file);
    graphic->addEntity(slide);
    redraw();
}

entity:
LC_Slide::LC_Slide(RS_EntityContainer *parent, const RS_Vector &d, const QString &file)
: RS_Point(parent, RS_PointData(RS_Vector(0.,0.))), pos(d), file(file) {}

RS2::EntityType LC_Slide::rtti() const{
    return RS2::EntitySlide;
}

RS_Entity *LC_Slide::clone() const{
    auto* s = new LC_Slide(*this);
    s->initId();
    return s;
}

void LC_Slide::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, pos.x, pos.y, fillBackground);

    slide_draw_qpainter(painter,
                        0,
                        0,
                        pos.x,
                        pos.y,
                        darkBackground,
                        qUtf8Printable(file));
}
Unfortunately, this doesn't work with the entity. There are two problems:

1. >> RS_PointData(RS_Vector(0.,0.))) must be in the drawing area, otherwise LC_Slide::draw(painter) won't execute.
This won't work if 0,0 isn't in the drawing area. But I don't have a coordinate from the user input.
The query doesn't seem to exist with RS_Preview.

2. The UCSMark is now visible as if in a drawing.
That doesn't happen with RS_Preview; I draw it over it.

I'm afraid this can only be solved with a custom container :-(.


entity.png

well, the preview is updated indeed.

But you may just override , like

void RS_PreviewActionInterface::mouseMoveEvent(QMouseEvent *event) {
    int status = getStatus();
    LC_MouseEvent lcEvent = toLCMouseMoveEvent(event);
//    deletePreviewAndHighlights();
    onMouseMoveEvent(status, &lcEvent);
  //  drawPreviewAndHighlights();
}

And probably suspend() and resume() methods in your action - and control the moment when preview is created/deleted by yourself.

Yet if the slide is not read from the file on each re-draw, and if the amount of entities will be not so big within the slide (so the drawing is fast) - even using the current implementation of preview will work.

However, if you'd like to use the direct implementation from the slide library - that's will not work, as
current implementation provided by the library is flawed, as it mixes loading and painting as part of the same operation.  

int slide_draw_qpainter(QPainter *painter,
                     unsigned x,
                     unsigned y,
                     unsigned width,
                     unsigned height,
                     bool darkbg,
                     const char *slide_uri)
{
    try {
        // Load slide.
        auto maybeSlide = slide_from_uri(slide_uri);


Probably there is less messy equivalient of this functiton, however...

so I could recommend you do the following:

1) create a new entity, say, LC_Slide, that will be inherited from RS_Entity.  
2) That LC_Slide holds the instance of the slide and implements draw() method the way that is similar to slide_draw_qpainter() yet without loading from file.
3) Within the action, as soon as the dialog is closed, create such entity and load if from the file, store the reference to in in a member field of the action class, if needed.
4) Add clone of the entity to preview where necessary.
5) Delete that entity as soon as as the action is destructed.

With such approach, the current implementation will work quite fine.

emanuel wrote

I have now set it up as described as an action with its own entity:

yes, the approach you've illustrated will not work indeed (will, actually it works as expected, yet in a different way that you'd like to achieve).

The reason for this - you try to add the entity to the container (i.e document).
In such case, it will be behave as any other entity and all rules for coordinates clipping etc) will be applied to it.

That describes the point 1) and 2)

So instead of adding to graphic, you need to add to overlay container (preview or highlight).
These containers (unlike to the drawing) are temporary and not persistent.
Drawing document - it's a document, it may be saved to DXF.

But again -   slide_draw_qpainter() is suxx, don't use it directly as it do both loading AND draw. It does not suitable for painting...

p.s as for draw()

void LC_Slide::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() * 5

Probably you don't need that bold fragment if you don't need to access settings. And in general, it's better to avoid using settings in "draw()" methods as they may be called really often.

The more optimal solution will be s just have a fields for bg color in LC_Slide class, and initialize it via setter or constructor at the moment of LC_Slide creation or so

That was a real pain!

I tried to build an LC_SlideData for LC_Slide, but the QString crashes in LC_Slide::draw(RS_Painter* painter) deep within QT when converting qUtf8Printable(file). I've spent hours testing with shifting types and variables. I have no idea what's causing this. Is QPainter a diva?

Even moving parts of the slidelib into the action didn't help at all to separate loading and rendering. The library is really poorly designed with lots of shared ptr consts.

I tried to incorporate as many of the tips as possible. And now I have a good Frankenstein solution (on shot preview).

The tip to overwrite the function actually provided a solution:

void LC_ActionFileViewSlide::resume()
{
    if (getStatus() == SetSlide)
    {
        setStatus(ShowSlide);
        RS_ActionInterface::resume();
    }
}

Now LC_Slide::draw(RS_Painter* painter) is executed exactly once.
I've also outsourced the settings.

void LC_ActionFileViewSlide::mouseReleaseEvent(QMouseEvent* e)
I still have in it.

https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/actions/file/lc_actionfileviewslide.h
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/actions/file/lc_actionfileviewslide.cpp

https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/lib/engine/overlays/slide/lc_slide.h
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/lib/engine/overlays/slide/lc_slide.cpp

RS_GraphicView::killShownActions() kills the action:
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/ui/qg_actionhandler.cpp#L473

yes, but it seems that the only proper way is just to modify the lib.
otherwise,  it's necessary to bother with ensuring one-time drawing. I suspect that just plain painting of the slide should be cheap, but file opening, reading and parsing is the most expensive part. Without it - it seems it will fit into overall framework quite well.

But in general, everything is much simpler regarding the action. The following snippets illustrates the code (I didn't compile it, so this is rather illustration).

1) LC_SlideEntity.h

class LC_Slide: public LC_OverlayDrawable
{
public:
    LC_Slide(const RS_Vector &d, const QString &file);
    void draw(RS_Painter* painter) override;
private:
    RS_Vector pos;
    QString file;
    RS_Color fillBackground;
    bool darkBackground = true;
    std::optional<std::shared_ptr<const Slide>> slideData;
};

2) LC_Slide.cpp

LC_Slide::LC_Slide(const RS_Vector& d, const QString& file)
    : pos(d), file(file), darkBackground(true) {
    LC_GROUP_GUARD("Colors");
    auto bgc = QColor(LC_GET_STR("background", RS_Settings::background));
    fillBackground = bgc;
    if ((bgc.red() * 299 + bgc.green() * 587 + bgc.blue() * 114) / 1000 >= 125) {
        darkBackground = false;
    }
    slideData = slide_from_uri(slide_uri);
    if (!slideData) {
        std::ostringstream ss;
        ss << "Slide " << slide_uri << " not found";
        throw std::runtime_error{ss.str()};
    }
}

void LC_Slide::draw(RS_Painter* painter) {
    RS_DEBUG->print("LC_Slide::draw");
    painter->fillRect(0, 0, pos.x, pos.y, fillBackground);
    auto slide = slideData.value();
    unsigned sld_width = slideData->header().high_x_dot();
    unsigned sld_height = slideData->header().high_y_dot();
    double sld_ratio = slideData->header().aspect_ratio();
    // Draw slide.
    SlideRecordsVisitorQPainterDrawer visitor{
        painter,
        sld_width, sld_height,
        sld_ratio,
        x, y,
        width, height,
        darkbg,
    };
    slide->visit_records(visitor);
}

3) Action LC_ActionFileViewSlide.h

class LC_ActionFileViewSlide : public RS_PreviewActionInterface {
    Q_OBJECT
public:
    LC_ActionFileViewSlide(RS_EntityContainer& container, RS_GraphicView& graphicView);
    void init(int status) override;
    void mouseMoveEvent(QMouseEvent* e) override;

protected:
    enum Status {
        SetSlide,       /**< Setting Slide */
        ShowSlide       /**< Showing Slide */
    };
    void createSlideEntity(const QString &file);
    void prepareSlide();
    void clear();
    void onMouseMoveEvent(int status, LC_MouseEvent* event) override;
    void onMouseRightButtonRelease(int status, QMouseEvent* e) override;
private:
    LC_Slide *slide;
    bool addedToPreview;
};

4)  lc_slide implemenation

LC_ActionFileViewSlide::LC_ActionFileViewSlide(RS_EntityContainer& container,
                                               RS_GraphicView& graphicView)
    : RS_PreviewActionInterface("View slide", container, graphicView) {
    setActionType(RS2::ActionFileViewSlide);
}

void LC_ActionFileViewSlide::init(int status) {
    RS_ActionInterface::init(status);
    if (status > 0) {
        prepareSlide();
    }
    else {
        delete slide;
    }
}

/*
 * This method is just an illustration of using mouse move for persistent preview (one that is not cleared)
 * yet it is necessary to check - probably it will be necessary to override resume too...
 */
// void LC_ActionFileViewSlide::mouseMoveEvent(QMouseEvent* e) {
//     if (getStatus() == ShowSlide) {
//         /* this is variant for persistent preview*/
//         if (!addedToPreview) {
//             previewEntity(slide);
//         }
//     }
// }

/**
 * if method is not overriden, preview will be deleted and created again
 */
void LC_ActionFileViewSlide::mouseMoveEvent(QMouseEvent* e) {
    RS_PreviewActionInterface::mouseMoveEvent(e);
}

void LC_ActionFileViewSlide::onMouseMoveEvent(int status, LC_MouseEvent* event) {
    // just add clone of slide to the preview for displaying.
    // clone will be deleted by preview container.
    previewEntity(slide->clone());
}

void LC_ActionFileViewSlide::createSlideEntity(const QString& file) {
    RS_DEBUG->print("LC_ActionFileViewSlide::drawOverlaySlide file: %s", qUtf8Printable(file));

    /*
     * What if the size of the window and so the graphicview will be changed after this!?
     * I.e - window change while the action is active?
     * In general, sized should not be shown there, but handled by the implementation of the entity.
     */
    slide = new LC_Slide(RS_Vector(graphicView->getWidth(), graphicView->getHeight()), file);
}

void LC_ActionFileViewSlide::prepareSlide() {
    if (graphic != nullptr) {
        QString filename = QFileDialog::getOpenFileName(NULL,
                                                        tr("open slide"),
                                                        "",
                                                        "AutoCAD Slide (*.sld)");

        if (filename.isEmpty()) {
            LC_ERR << __func__ << "(): empty file name, no Slide is selected";
            setStatus(-1);
            return;
        }
        createSlideEntity(filename);
        setStatus(ShowSlide);
    }
}

void LC_ActionFileViewSlide::onMouseRightButtonRelease(int status, QMouseEvent* e) {
    setStatus(-1);
}

The LC_Slide now works with a few adjustments.
https://github.com/emanuel4you/LibreCAD/blob/developer/librecad/src/lib/engine/overlays/slide/lc_slide.cpp

Unfortunately, the PreviewAction doesn't:

my clone:
LC_OverlayDrawable* LC_Slide::clone() const {
    LC_Slide* s = new LC_Slide(*this);
    return s;
}

../../../librecad/src/actions/file/lc_actionfileviewslide.cpp: In member function ‘virtual void LC_ActionFileViewSlide::onMouseMoveEvent(int, LC_MouseEvent*)’:
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:139:31: error: cannot convert ‘LC_OverlayDrawable*’ to ‘RS_Entity*’
  139 |     previewEntity(slide->clone());
      |                   ~~~~~~~~~~~~^~
      |                               |
      |                               LC_OverlayDrawable*
In file included from ../../../librecad/src/actions/file/lc_actionfileviewslide.h:31,
                 from ../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:25:
../../../librecad/src/lib/actions/rs_previewactioninterface.h:121:35: note:   initializing argument 1 of ‘void RS_PreviewActionInterface::previewEntity(RS_Entity*)’
  121 |     void previewEntity(RS_Entity *en);
      |                        ~~~~~~~~~~~^~
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:136:51: warning: unused parameter ‘status’ [-Wunused-parameter]
  136 | void LC_ActionFileViewSlide::onMouseMoveEvent(int status, LC_MouseEvent* event) {
      |                                               ~~~~^~~~~~
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:136:74: warning: unused parameter ‘event’ [-Wunused-parameter]
  136 | void LC_ActionFileViewSlide::onMouseMoveEvent(int status, LC_MouseEvent* event) {
      |                                                           ~~~~~~~~~~~~~~~^~~~~
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp: In member function ‘virtual void LC_ActionFileViewSlide::onMouseRightButtonRelease(int, QMouseEvent*)’:
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:170:60: warning: unused parameter ‘status’ [-Wunused-parameter]
  170 | void LC_ActionFileViewSlide::onMouseRightButtonRelease(int status, QMouseEvent* e) {
      |                                                        ~~~~^~~~~~
../../../librecad/src/actions/file/lc_actionfileviewslide.cpp:170:81: warning: unused parameter ‘e’ [-Wunused-parameter]
  170 | void LC_ActionFileViewSlide::onMouseRightButtonRelease(int status, QMouseEvent* e) {
      |                                                                    ~~~~~~~~~~~~~^
make[2]: *** [Makefile:27930: ../../generated/librecad/obj/lc_actionfileviewslide.o] Error 1

yes, and that's actually reasonable, since it seems that LC_Slide is put into overlay. Sorry, I forgot about drawing in overlays.  

Just inherit LC_Slide not from RS_Entity but from LC_OverlayDrawable. LC_OverlayDrawable is actually a more lightweight notion that the normal entity, and it's intended for drawing mainly (in overlays).

hm... it seems that the issue you've had in the action is due to this code:

    LC_OverlayDrawablesContainer *drawablesContainer = viewport->getOverlaysDrawablesContainer(RS2::OverlayGraphics::ActionPreviewEntity);
    drawablesContainer->add(sl);

If you'd like to put something directly into overlay - you'll need to inherit from LC_OverlayDrawable.

But if you interited from RS_Entity (as currently) - just use previewEntity(), as I've pointed in my implementation of the action earlier.

The only thing is needed is

void LC_ActionFileViewSlide::onMouseMoveEvent(int status, LC_MouseEvent* event) {
    // just add clone of slide to the preview for displaying.
    // clone will be deleted by preview container.
    previewEntity(slide->clone());
}

Try to rely on the logic of the action I've posted.

LC_OverlayDrawablesContainer *drawablesContainer = viewport->getOverlaysDrawablesContainer(RS2::OverlayGraphics::ActionPreviewEntity);
drawablesContainer->add(sl);

That worked fine with ActionInterface.

I had tried the src you posted.

I didn't want to rebuild the LC_Slide as an RS_Entity, so it wouldn't render if the point wasn't in the drawing area.
What I can test with the new src PreviewAction code is the implemented addOverlay().

hm.. if you put the entity to the drawing - yes, indeed, it will not render if point is not in the drawing area.

Yet if you put the entity into preview as container (via previewEntity()) - the clipping may be also adjusted there.

But if you just would like to how the slide on top of the everything - yes, inheriting from LC_OverlayDrawable instead of RS_Entity, and using the overlay is the simplest method indeed.