From 9ab26458585e61645805be4582783971fb45d469 Mon Sep 17 00:00:00 2001 From: wind1900 Date: Wed, 25 Dec 2024 10:28:12 +0800 Subject: [PATCH] [20_13] Do not add duplicated values to copy_always to avoid stack overflow ## What Add check before adding new values to `edit_interface_rep::copy_always` in function `edit_interface_rep::apply_changes()`. New rectangle will be added to `copy_always` if `copy_always` is empty/nil or the first item of `copy_always` is not equal to newly added value. ## Why Since `apply_changes()` will be called when the window receives any event, `copy_always` will become very large and cause stack overflow when it's cleared. The `copy_always` is a linked list and when it's cleared, the destructor is called from head to tail recursively, causing stack overflow. ## How to test your changes? That's the potential risk: I can only test it on Windows with few scenarios, I don't know the result in Linux, Mac. already run `xmake run --yes -vD --group=tests` and `xmake run --yes -vD --group=integration_tests` The investigations below are all in Windows environment. ### Where is the code to clear `copy_always`? In `edit_interface_rep::scroll_to()` and `edit_interface_rep::set_extents()`, by using `copy_always = rectangles ()`. ### When will `copy_always` be cleared? `scroll_to()` is called when the cursor is moved out of current view. I can't add breakpoint in `set_extents()`, I don't know when will it will be called. ### What's the purpose of `copy_always`? The comment says it's used for wiping out cursor. However I think it doesn't do anything now. I comment out the code that uses it, get nothing wrong. But removing it is more dangerous. ### Where is `copy_always` used? In `edit_interface_rep::draw_pre()` and `edit_interface_rep::draw_with_shadow()`. In `draw_pre()`, it will go through all rectangles in `copy_always` and call `win->put_shadow()`, but it doesn't draw anything at all. Because in `qt_renderer_rep::put_shadow()`, `painter == static_cast (ren)->painter` is true and returns directly. In `draw_with_shadow()`, it's only used if `gui_interrupted()` returns true, but `gui_interrupted()` doesn't return true in my machine. These shadow functions are related to the double buffering, the behavior may be different in other OS. In summary, I don't think it's necessary to store all the old rectangles in `copy_always`. The latest ones `ocr`(old cursor) and `ncr`(new cursor) should be enough for cursor drawing stuff. --- src/Edit/Interface/edit_interface.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Edit/Interface/edit_interface.cpp b/src/Edit/Interface/edit_interface.cpp index 06a960032..3e2b2f2aa 100644 --- a/src/Edit/Interface/edit_interface.cpp +++ b/src/Edit/Interface/edit_interface.cpp @@ -832,15 +832,19 @@ edit_interface_rep::apply_changes () { oc->oy + (oc->y1 - dw) - P3, oc->ox + ((SI) ((oc->y2 + dw) * oc->slope)) + P2 + dw, oc->oy + (oc->y2 + dw) + P3); - copy_always= rectangles (ocr, copy_always); + if (is_nil (copy_always) || copy_always->item != ocr) { + copy_always= rectangles (ocr, copy_always); + } invalidate (ocr->x1, ocr->y1, ocr->x2, ocr->y2); rectangle ncr (cu->ox + ((SI) ((cu->y1 - dw) * cu->slope)) - P3 - dw, cu->oy + (cu->y1 - dw) - P3, cu->ox + ((SI) ((cu->y2 + dw) * cu->slope)) + P2 + dw, cu->oy + (cu->y2 + dw) + P3); invalidate (ncr->x1, ncr->y1, ncr->x2, ncr->y2); - copy_always= rectangles (ncr, copy_always); - oc = copy (cu); + if (is_nil (copy_always) || copy_always->item != ncr) { + copy_always= rectangles (ncr, copy_always); + } + oc= copy (cu); // set hot spot in the gui send_cursor (this, (SI) floor (cu->ox * magf), (SI) floor (cu->oy * magf));