s25edit backports from s25client by morganchristiansson · Pull Request #37 · Return-To-The-Roots/s25edit

morganchristiansson · 2026-06-16T15:03:24Z

~~Allow window resizing~~ split to Allow window resizing #46
Fix segfault with small map&large screen
Fix black area with small map&large screen
Fix mouse panning on Linux
Zoom with mouse wheel

Flamefire

Thanks for the PR, that definitely improves it!
Some notes inline, a common theme is that any pair of x/y or width/height values should use either Point/PointF or Extent which avoids C&P errors and makes processing easier (single assignment instead of 2, operator support for combining with other Points)
Note: In some places fixed-size variants are used. That is historical and not necessary in almost all cases, so no need to introduce new uses of them unless there is a specific reason for the size requirement.

Thanks again!

Flamefire · 2026-06-16T15:30:50Z

        moveMap(offset);

-        // this whole "warping-thing" is to prevent cursor-moving WITHIN the window while user moves over the map
-        SDL_EventState(SDL_MOUSEMOTION, SDL_IGNORE);


Why the change? Isn't handling this "issue" here better where it is caused than where mouse moves should be handled?
IIRC SDL offers a relative mouse mode. As we only support SDL here that might be the best solution if we change this

This is matching current s25client VideoSDL2.cpp which doesn't use SDL_EventState.

That is not a good reason to me.

Well they're clearly not needed.

Re: removing SDL_EventState(SDL_MOUSEMOTION, SDL_IGNORE)

This change is part of what fixes mouse panning on Linux in this PR. The old SDL_EventState/SDL_IGNORE/SDL_ENABLE dance is a global filter that temporarily drops all motion events — on > Linux (X11/Wayland) the warp-generated event can arrive asynchronously after SDL_ENABLE is called, creating a feedback loop where each motion event warps the mouse, which generates > another event, which warps again, etc. That matches the symptom of panning that stutters and becomes unusable.

The replacement filters by position identity: a motion event with the same coordinates as the previous one is a warp echo and gets skipped. It's strictly more targeted, and it converges > immediately instead of looping.

I know "s25client does it" isn't a strong argument on its own — I mention it more as validation that this approach is well-tested in production, not as the reason for the change. The actual reason is that it fixes a real bug with a more precise filter, replacing a global side-effect that interacts poorly with asynchronous event delivery on Linux.

So this is the exact source of frustration causing mouse scrolling to sputter and fail after just a few frames in every PR I work on.

morganchristiansson · 2026-06-16T21:05:28Z

Maybe I should split this into smaller PRs.

I was working on import heightmap as I posted about on Discord, not sure I will submit as PR. And these were some quality of life improvements that felt important.

morganchristiansson · 2026-06-20T12:01:59Z

What's the holdup here? All review comments are resolved.

Flow86 · 2026-06-20T12:23:50Z

What's the holdup here? All review comments are resolved.

life ;-)

oh and there is a conflict

morganchristiansson · 2026-06-20T22:52:44Z

Ah ok no worries then. I'm just impatient lol.
There's no conflicts? All green.
I based other branches on this one and can't submit them for PRs until this one is merged. Mainly my s25edit OpenGL branch which would be super nice to get in for review it makes a huge difference.

morganchristiansson · 2026-06-30T11:03:05Z

In every branch I work on I'm frustrated by the lack of the Fix mouse panning commit that's in this PR.

Flamefire · 2026-06-30T11:18:07Z

oh and there is a conflict

morganchristiansson · 2026-06-30T12:04:58Z

I went all in on rebase. Squashed some commits with feedback into original commit, removed the commit that was reverted and extracted.
There were conflicts from master.
Please give it another review pass. Hope it's cleaner now tho.

morganchristiansson · 2026-06-30T12:06:08Z

 #include "callbacks.h"
 #include "globals.h"

+namespace {


Why am I using anonymous namespaces like this?

Please advice, my review overlords.

Ok it's just file local helper function that is not available from other object files.
So it's fine. Not needed but whatever.
I'd assumed that not being added to .h would've made it inaccessible but I guess this is explicit and goes further.
AI adds these on it's own initiative and then makes a mess and chokes on it when there's multiple namespace scopes in same file.

morganchristiansson · 2026-06-30T12:13:17Z

Also renamed commit "Wrap mouse for large screens" ->"Wrap brush for large screens", as it refers to the icons drawn on vertices not the cursor itself.
And re-ordered height, width to width height to match x, y order. Confusing otherwise.

Flamefire · 2026-06-30T12:37:22Z

-        Xeven += (map->width);
-    else if(Xeven > map->width - 1)
-        Xeven -= (map->width - 1);
+    Xeven = ((Xeven % map->width) + map->width) % map->width;


Can you add a function for that please? Something like int positiveIntegerModulo(int,int) or see if your AI can come up with a better name. I'm worried about repeating the same value 3 times per line which at least makes review hard as there might be a C&P mistake somewhere

Flamefire requested changes Jun 16, 2026

View reviewed changes

morganchristiansson force-pushed the resize-window branch from 67f2175 to 604b117 Compare June 16, 2026 16:11

This was referenced Jun 18, 2026

Change x,y->Point w,h->Extent #40

Merged

Tidy zoom feature that was marked experimental #41

Merged

morganchristiansson requested a review from Flamefire June 18, 2026 17:02

This was referenced Jun 18, 2026

s25edit PR feedback: Single case switch -> if Return-To-The-Roots/s25client#1954

Merged

/usr/bin/s25edit has poor performance on linux #22

Open

can't use mouse and right click to scroll through the map #33

Open

This was referenced Jun 22, 2026

Allow window resizing #46

Merged

Make the editor window resizable and fill it with the (wrapping) map #51

Open

morganchristiansson added 5 commits June 30, 2026 14:01

Wrap brush for large screens

67c147c

Black area for small maps&large screens

7b71843

Fix mouse panning

511324b

Zoom with mouse wheel

05fcd18

Fix segfault when panning map

74d768d

morganchristiansson force-pushed the resize-window branch from fc01c45 to 74d768d Compare June 30, 2026 12:03

morganchristiansson commented Jun 30, 2026

View reviewed changes

fixup! Zoom with mouse wheel

20cd336

Flamefire reviewed Jun 30, 2026

View reviewed changes

positiveModulo

9694bea

morganchristiansson force-pushed the resize-window branch from 420fbe3 to 9694bea Compare July 6, 2026 18:26

Uh oh!

Conversation

morganchristiansson commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Flamefire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Flamefire Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

morganchristiansson Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

Flamefire Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

morganchristiansson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

morganchristiansson Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morganchristiansson Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

morganchristiansson commented Jun 16, 2026

Uh oh!

morganchristiansson commented Jun 20, 2026

Uh oh!

Flow86 commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morganchristiansson commented Jun 20, 2026

Uh oh!

morganchristiansson commented Jun 30, 2026

Uh oh!

Flamefire commented Jun 30, 2026

Uh oh!

morganchristiansson commented Jun 30, 2026

Uh oh!

morganchristiansson Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

morganchristiansson Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morganchristiansson commented Jun 30, 2026

Uh oh!

Flamefire Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

morganchristiansson commented Jun 16, 2026 •

edited

Loading

morganchristiansson Jul 6, 2026 •

edited

Loading

Flow86 commented Jun 20, 2026 •

edited

Loading

morganchristiansson Jun 30, 2026 •

edited

Loading