Skip to content

Commit

Permalink
[20_13] Do not add duplicated values to copy_always to avoid stack ov…
Browse files Browse the repository at this point in the history
…erflow

<!-- Thank you for your contribution! -->
## 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<qt_renderer_rep*> (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.
  • Loading branch information
wind1900 authored Dec 25, 2024
1 parent 0a2d37f commit 9ab2645
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/Edit/Interface/edit_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 9ab2645

Please sign in to comment.