[BUG][PLAYER] Zooming In Fails to Preserve Mouse-On-Land [FIXED]

[WillLem]

Happy to have a session if it helps you dig into the bug.

Mumble: I can make Saturday, 09:00 UTC. (This is in the morning, i.e., it's not 21:00 UTC.) Or I can make Sunday, 09:00 UTC.

Can you perhaps do sometime in the evening during next week? I'll be busy pretty much all weekend and probably won't be able to find any time for programming.

The following weekend would be OK if weekday evenings are no good for you.

Regarding screen sharing, as long as you can see my screen and we can hear each other, that should be good enough. You can direct me as you wish (I should be fairly quick to respond as I'm much more fluent with Delphi/SLX codebase by now).

It's about time we got this one sorted!

[Simon]

I'm on an odd rhythm: I sleep from late afternoon until around midnight. I have free time before work, and still start my day job before others appear online.

Catch me in Lix's IRC channel late at night. I can't guarantee that we'll find a good matching time, but if you have time, let's try. I'll sit in IRC around the clock, even when I'm working or sleeping; I won't reply instantly then.

I don't have screensharing ready yet. Normally no problem, I'll look at my own copy of the code, but the zoom bug in particular would enjoy screensharing.

-- Simon

[namida]

Given the recent decision to limit further work on NL, I'm not going to look at this for NL, however as there is active work on this for SLX, I've moved this topic to the SLX board rather than closing it.

[Simon]

New idea. We don't need to understand anything about the existing code. Instead:

• Remember which land pixel is under mouse.
  
• Zoom in/out with existing routine.
  
• Look at which land pixel is under mouse.
  
• Scroll map by difference of steps 1 and 3.
  

-- Simon

[WillLem]

New idea. We don't need to understand anything about the existing code. Instead:
(Idea)

Great idea. Code already exists to fully support what you've proposed and shouldn't be too difficult to harness. The only issue might be unpicking the existing zoom thread (if necessary), depending on how far it's spread through the codebase.

[WillLem]

After some investigation/experimentation, we've ascertained the following points of interest which may lead to an eventual solution:

1) The existing variable for tracking the cursor position relative to the level is fRenderInterface.MousePos. This TPoint stays the same for each zoom factor, and updates as expected when moving the cursor across the level.

2) In order to use fRenderInterface.MousePos in GameWindow's ChangeZoom procedure, we need to initialize the Render Interface before the call to ChangeZoom in TGameWindow.PrepareGameParams. N.B. This may mean having to initialize it twice (i.e. keeping the existing initialization, which is after the call to ChangeZoom). So, this might cause problems elsewhere...

3) Mouse.CursorPos is the mouse position relative to the screen, not the level (or even the game window).

4) Img.OffsetHorz and Img.OffsetVert are most likely the variables needed for repositioning the level relative to the cursor after zooming (should we choose to go ahead with that method of fixing this bug), but this is not yet certain.

[WillLem]

Great news! I reached out to Anders Melander (Graphics32 maintainer) and asked if there might be an existing solution to this problem within the GR32 library.

Turns out there is!

GR32_Image has a procedure simply titled Zoom which can specify a Pivot point which anchors the image whilst zooming.

After a quick test, it seems to work a treat. The only issue I can find with it is that the cursor is repositioned when zooming between factor 1 and factor 2 (this is most pronounced at the extreme level edges/corners). Between factor 2 and higher, cursor position is preserved perfectly. I'm not sure exactly what's going on here, but it's definitely a promising start

For clarity, in this context "factor" is defined by GameWindow's fInternalZoom field - this may not be quite the same value as the GR32 definition (actually, if the values are different for any reason, that could be the source of this bug).

@Simon - I've committed the new ChangeZoom procedure as provided by Anders. From here, we can likely find out how to fix 1-2 zooming. Once that's done, I'll create a PR for NeoLemmix.

[WillLem]

After some investigation, observation of Lix's zooming mechanics, and insight from Anders, it seems that the two display goals of "keep everything as visible as possible" and "zoom in to a very specific point under the mouse cursor" are mutually incompatible below a certain zoom factor, whatever the system's logic.

For example, even in Lix, the cursor jumps away from the edge when zooming in from the lowest possible zoom factor:


Lix - note that the cursor jumps away from both edges when zooming in from the very lowest zoom factor


And, the same behaviour in SuperLemmix

Anders observed that, at least in NL/SLX, this behaviour is most likely due to display logic that wants to display as much of the gameplay screen as possible at all times, with the zoom pivot only being "accepted" when the zoomed-in game display reaches a certain size (i.e. larger than the display window itself). This is unlikely to be something that we want to mess about with too much, so we must accept some loss of pivot when zooming in to/from the very lowest factor.

In practice, this actually isn't too much of an issue. Pinpoint zooming becomes more useful/relevant at higher zoom factors, where the image being displayed is larger than the display window and we want to get as close as possible to a particular lemming or level item. Both Lix and now SuperLemmix have acceptable levels of cursor re-adjustment through the very lowest zoom factors, whilst anchoring perfectly throughout the higher zoom factors:


Lix - note that it's necessary to readjust the cursor between the first few zoom factors


And, the same for SuperLemmix - only 1 adjustment is necessary because the game window is lower resolution than Lix's. However, on larger-resolution displays, more adjustments may be necessary before the anchor takes effect

Furthermore, both programs tend to display the level at a higher zoom factor than the lowest possible by default, so it's usually necessary to first decrease the zoom in order that it reaches a pivot-incompatible factor in the first place.

So, I'd suggest closing this as resolved unless anyone has any specific ideas. Although not actually intended as a final solution to the problem, Anders' fix has the zoom behaving much better than previously and is definitely a step in the right direction. It may of course need further testing to ensure stability, but from what I can see so far it seems perfectly usable.

SuperLemmix implements this fix in Commit 1fe01cea0

[Simon]

Lix
And, the same for SuperLemmix

Excellent work! And thanks for making these showcasing gifs!

Your screenshots show that, yes, when zooming between deep-enough zoom factors, mouse-on-land is now preserved. This fixes the main bug.

I'll happily test this in SLX. Let me know when you have an SLX executable for testing.

"keep everything as visible as possible" and "zoom in to a very specific point under the mouse cursor" are mutually incompatible below a certain zoom factor, whatever the system's logic.

Correct.

The main bug is how NL's/SLX's zoom behave when the zoom factors both before and after fill the entire screen, i.e., on both levels, there is more level in both x- and y-direction than would fit onto the screen. This is the case in which I require that both mouse-on-screen and land-under-mouse are preserved.

I'd suggest closing this as resolved unless anyone has any specific ideas. Although not actually intended as a final solution to the problem, Anders' fix has the zoom behaving much better than previously and is definitely a step in the right direction.

Right, there is possibly more here.

geoo suggested to me in February 2023 for Lix: When the mouse is over the land (i.e., not over the void outside the land), I should still preserve land-under-mouse and mouse-on-screen. He's happy with sacrificing the goal of keeping as much as possible visible, i.e., preservation of land-under-mouse is more important than raising the screen's ratio of land-to-void.

I believe geoo is right here, but I haven't implented it in Lix yet. Lix's code for drawing the colored boundaries over the void is heavily tailored to rendering the land at the bottom center of the screen. geoo's idea will also procude new scrolling logic: You can have void showing at, e.g., the right, even though there is more unshown land to the left, and you can scroll left from here, but then not back to the right.

I'd say: Before you dig deeper into this idea (which is likely good, but not certainly), it's my turn again to improve/test this in Lix first.

See also my post from 2023-02-07 and Lix github #460: Preserve Mouse-on-Land even when zoomed-out.

-- Simon

[Simon]

I'll have time to test this weekend (Sat 9 to Sun 10). Please build it for me to test!

-- Simon

[Simon]

WillLem has sent me an NL testing executable via PM.

The zoom works well now, exactly as expected: It preserves both the mouse on land and the mouse on screen,  as WillLem has described. Now we really zoom into and out of the land pixel over which the mouse hovers. I haven't found any bugs nor rounding errors.

If NL can shift more land into view (and shift void out of view), NL does so, i.e., it prefers to show as much land as possible over fixing mouse-on-land. This is the same behavior as before; both Lix and NL have chosen this tradeoff for years.

This is exactly what I suggested. Thank you very much!

And you've made the implementation shorter and easier.

-- Simon

[WillLem]

Glad this fix has your approval, Simon!

@namida In case you want to merge this into NL 12.14, here's the necessary code exactly as sent by Anders. It's an easy fix so not really worth sending a PR (although I'm happy to do so if you'd prefer). Simply replace GameWindow's existing ChangeZoom procedure with this one:

Code Select Expand

procedure TGameWindow.ChangeZoom(aNewZoom: Integer; NoRedraw: Boolean = false);
var
  Pivot: TPoint;
begin
  aNewZoom := Max(Min(fMaxZoom, aNewZoom), 1);
  if (aNewZoom = fInternalZoom) and not NoRedraw then
    Exit;

  SkillPanel.Image.BeginUpdate;
  try
    Pivot := Img.ScreenToClient(Mouse.CursorPos);
    Pivot := Img.ControlToBitmap(Pivot);
    Img.Zoom(aNewZoom, Pivot);

    fInternalZoom := aNewZoom;

    ApplyResize(False);

    SetRedraw(rdRedraw);
  finally
    SkillPanel.Image.EndUpdate;
  end;
end;

[Simon]

Please make a proper PR for namida. You'll make it as easy for him as you can.

Rebase the lone commit onto his master. You can remove "zoom factor 2 and higher, but 1-2 still has repositioning issues. Currently investigating..." from the rebased commit's message. You have the theoretical answer: NL prefers to shift more land into view.

-- Simon

[namida]

I'm going to leave it. I thought at first, I'll include it if it's well tested - but, ideally, 12.14.0 will be the final version, and especially I don't want to be last-minute introducing a UI change, thus either extending the RC phase (especially when some users have content specifically ready for when it goes stable) or having the potential of creating a situation of "new minor but annoying (or worse) bug is introduced in what was meant to be the final version, and a further update becomes necessary to either fix the issue or revert the behavior altogether".

By all means though - you've mentioned the idea of making a "community edition" fork of NL that maintains full 12.14.0 physics compatibility but adds / improves UI stuff. This would be a perfect fit for that.

[WillLem]

By all means though - you've mentioned the idea of making a "community edition" fork of NL that maintains full 12.14.0 physics compatibility but adds / improves UI stuff. This would be a perfect fit for that.

@Simon - A PR most likely wouldn't have made the difference here; I totally understand Namida's reasons for not wanting to include this feature.

Let's resolve now to go ahead with 12.14 CE when the time comes. I can include this and any other UI upgrades that the community might want.