Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some menus are blank #71994

Closed
IdleSol opened this issue Feb 26, 2024 · 35 comments · Fixed by #72035
Closed

Some menus are blank #71994

IdleSol opened this issue Feb 26, 2024 · 35 comments · Fixed by #72035
Labels
[C++] Changes (can be) made in C++. Previously named `Code` ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds Info / User Interface Game - player communication, menus, etc. (P2 - High) High priority (for ex. important bugfixes) (S2 - Confirmed) Bug that's been confirmed to exist

Comments

@IdleSol
Copy link
Contributor

IdleSol commented Feb 26, 2024

Describe the bug

After update #65709. @katemonster33

Two versions were compared:

See the screenshots for details

Attach save file

N/A

Steps to reproduce

N/A

Expected behavior

N/A

Screenshots

11
12
13
14
15
16

Versions and configuration

cdda-windows-tiles-x64-2024-02-26-0558

Additional context

If I find anything else, I'll add to the posts.

@IdleSol IdleSol added the (S1 - Need confirmation) Report waiting on confirmation of reproducibility label Feb 26, 2024
@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

When trying to select an AR-15

1

The game is running through Portproton (ArchLinux)

  • OS: Windows
    • OS Version: 10.0.18362 (21H1)
  • Game Version: 6e1e928 [64-bit]
  • Graphics Version: Tiles
  • Game Language: English [en]
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]

UPD. The error is being reproduced.

On the version (cdda-windows-tiles-x64-2024-02-26-0341) the game gives a standard error window.
errors

@NetSysFire NetSysFire added the ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds label Feb 26, 2024
@NetSysFire
Copy link
Member

I think your very first screenshot shows an existing potential "inconsistency" that may be unaffected by the recent imgui addition where the version is placed after any ascii art that may be displayed

@NetSysFire NetSysFire added the Info / User Interface Game - player communication, menus, etc. label Feb 26, 2024
@katemonster33
Copy link
Contributor

I've confirmed most of these screenshots, What view are you in when you try to "select an AR-15"?

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

What view are you in when you try to "select an AR-15"?

  1. Open the debug menu
  2. Spawn an item
  3. Enter "ar-15" in the search bar

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

I think your very first screenshot shows an existing potential "inconsistency" that may be unaffected by the recent imgui addition where the version is placed after any ascii art that may be displayed

Maybe. In an earlier version (0341), the drawing didn't show up at all. Or I was unlucky. So I'm not sure what the correct way is.

@NetSysFire
Copy link
Member

Or I was unlucky.

It is rng-based. That ascii art only shows up sometimes.

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

@katemonster33
In the game itself, I haven't seen any problems with the interface. But I was only looking at the Shift+1..0, -, = windows.

@katemonster33
Copy link
Contributor

I've narrowed it down to this commit causing the issue f37380e maybe @db48x has some wisdom? hate to just revert the commit

@NetSysFire NetSysFire added (S2 - Confirmed) Bug that's been confirmed to exist [C++] Changes (can be) made in C++. Previously named `Code` and removed (S1 - Need confirmation) Report waiting on confirmation of reproducibility labels Feb 26, 2024
@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

1

This is not your mistake, it was like this in previous versions. But maybe you can fix it:
test

UPD. Alternative. Repeat the variant with the mutation window. Descriptions are displayed at the bottom instead of the side.

@db48x
Copy link
Contributor

db48x commented Feb 26, 2024

I've narrowed it down to this commit causing the issue f37380e maybe @db48x has some wisdom? hate to just revert the commit

If you remove this patch it will definitely cause lines of text to be left undrawn (it will erroneously think that they already have been drawn).

I’ll take a look though.

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 26, 2024

1

As well as other items from the "main menu" (press esc in the game):

  • 4 Auto pickup manager
  • 5 Auto notes manager
  • 6 Safe mode manager
  • 7 Distractions manager (see screenshot)
  • 8 Color manager

I didn't take screenshots for all of these points. They all have a header problem.

@ZhilkinSerg ZhilkinSerg pinned this issue Feb 26, 2024
@ZhilkinSerg ZhilkinSerg added the (P2 - High) High priority (for ex. important bugfixes) label Feb 26, 2024
@katemonster33
Copy link
Contributor

katemonster33 commented Feb 26, 2024

When trying to select an AR-15

1

The game is running through Portproton (ArchLinux)

So running on Wine? This didn't happen for me on Windows. Can you try on a native Linux build?

@db48x
Copy link
Contributor

db48x commented Feb 26, 2024

I can reproduce it. Curiously enough, we are drawing spaces on those lines that are missing.

@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

I haven't figured it out, but I have narrowed it down a bit. In user_interface::show in the file auto_pickup.cpp there is an on_redraw callback. This callback draws the autopickup manager window in three parts. The border is drawn into w_border, the header into w_header, and the rules inw. If I comment out everything after the call to wnoutrefresh( w_header );, then the header is correctly drawn. In fact, after drawing and refreshing w_header, it goes back and adds a scrollbar to w_border. If I comment out just the second call to wnoutrefresh( w_border ); then the header is also still visible. So it seems that refreshing this window is erasing the header.

And moving the call to draw_scrollbar up to before the header is ever drawn also fixes it. I suspect the other windows all have the same problem. We could fix them all by rearranging them to add the scrollbar earlier, or we could delve deeper into cursesport to see why it is erasing the header window (which is overlapping with the border window). I need to get some sleep, so I leave it to you @katemonster33. I’ll check back in tomorrow evening.

@katemonster33
Copy link
Contributor

I can reproduce it. Curiously enough, we are drawing spaces on those lines that are missing.

Can you print the stack trace?

@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

I guess it’s just because every window has its own line vector. It calls wnoutrefresh( w_header ); which draws w_header->line, then it calls wnoutrefresh( w_border ); which draws w_border->line over top of it.

@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

Another fix would be to give each scrollbar their own window that is only one column wide. Then it can be drawn any time you want without clearing the whole window. But that’s a more invasive change than just rearranging the order that windows are drawn in.

@katemonster33
Copy link
Contributor

I'm coming up with nothing. We may need to simply put the framebuffer back in for now and come up with some solution for letting it work with imgui... maybe invalidate the framebuffer on every redraw when imgui is on screen.

was hoping we could just remove that commit temporarily and make a new draft PR with the commit to fix master for now.... waiting on an answer to see if that is possible.

@Qrox
Copy link
Contributor

Qrox commented Feb 27, 2024

I guess before removing the framebuffer the spaces in those windows are skipped due to the framebuffer not being invalidated because nothing was printed there, so removing the framebuffer causes all spaces to be drawn regardless of whether the location is invalidated. Maybe instead of clearing the window area before drawing a window, only erase locations in the window that have been printed to? Or perhaps revert the removal and properly invalidate the area used by ImGUI windows, IDK.

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 27, 2024

So running on Wine? This didn't happen for me on Windows. Can you try on a native Linux build?

Unfortunately, I can't. The available version is from January 9 (0.G.2024.01.09-1). And for building from source, I don't have enough knowledge.

Windows loaded in a virtual machine.
1

If I can get to another computer with normal windows. I'll try to check there.

@NetSysFire
Copy link
Member

There are available builds for Linux built every day, e.g cdda-linux-tiles. https://github.com/CleverRaven/Cataclysm-DDA/releases

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 27, 2024

Here is the result on another windows 10 computer.

  • OS: Windows
    • OS Version: 10.0.19045.4046 (22H2)
  • Game Version: 6e1e928 [64-bit]
  • Graphics Version: Tiles
  • Game Language: English [en]
  • Mods loaded: [
    Dark Days Ahead [dda],
    Disable NPC Needs [no_npc_food],
    Portal Storms Ignore NPCs [personal_portal_storms],
    Slowdown Fungal Growth [no_fungal_growth]
    ]
    1
    Same as on the virtual machine

If you press alt+f4 while the game is loading:
2
Once the window is closed, a second window will appear:
3

I've been paying attention. In my settings: Use ImGui UI: false. Doesn't that mean everything stays as it was? Why did I have all these errors with windows then?

Enabling the UI, does not change the window header errors.

@andrei8l
Copy link
Contributor

There are available builds for Linux built every day, e.g cdda-linux-tiles. https://github.com/CleverRaven/Cataclysm-DDA/releases

Linux builds are failing due to a library mismatch https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/8059264719/job/22016044229#step:17:369

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 27, 2024

Additional information.

If the game is run through PortProton, the error (with imGui) causes the game to crash.
If the game is run on Windows, the error (with imGui) does not cause a game crash (if you press skip).

This error (with imGui) occurs before the game error message. For example: #71451 and #71781

Calling the test error message, does not cause an error with imGui.

If you get a game error and click ignore error messages, then repeating the error, does not cause an error with imGui.

Example (win10):

  1. Open the debug menu
  2. Spawn an item s/w
  3. Enter "ar-15" in the search bar
  • There is an error with imGui: line 9113
  • Click skip
  • Get an error: "loaded a nun or ammo_data() is nullptr"
  • Press I
  • Сlose the spawn window
  1. Repeat steps 2 and 3. No errors

The problem is that not every window causes an error to appear with imGui.

For example, disconnecting a modification from the m4 carbine causes a "loaded a nun or ammo_data() is nullptr" error, but does not cause an imGui error.

@NetSysFire NetSysFire changed the title Some bugs with UI Some menus are blank Feb 27, 2024
@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

Ok, it is quite evident that there is more than one bug here. We need to split this up into multiple bug reports.

  • Separately report ammo_data/loaded nun errors (which is probably not ImGui related)
  • Separately report SDL version mismatch (now requires SDL >=2.0.17)
  • Separately report ImGui uninitialized errors on Windows (it is vital to include the crash log here, for the stack trace)

We’ll leave this bug report for the windows that are missing some lines, usually headers, but also sometimes descriptions.

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 27, 2024

Separately report ammo_data/loaded nun errors (which is probably not ImGui related)

The problem is not with this error. The problem is that there is an error with imGui before it.

@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

And #72022

@db48x
Copy link
Contributor

db48x commented Feb 27, 2024

At the moment I am inclined to simply look for all places where draw_scrollbar is called, and move that call up so that it happens just after the window border is drawn. It’s a little bit like rearranging the deck chairs on the Titanic, since all of that code will be rewritten in ImGui and ImGui will handle the scrollbars for us, but it’s an easy fix. I’ll do it myself this evening, unless someone else does something about it before then.

@db48x
Copy link
Contributor

db48x commented Feb 28, 2024

… Maybe instead of clearing the window area before drawing a window, only erase locations in the window that have been printed to? …

I was thinking about this all day and eventually realized that we could just clear the window line by line, skipping the lines that aren’t touched, instead of clearing the whole window in one go.

PR incoming.

db48x added a commit to db48x/Cataclysm-DDA that referenced this issue Feb 28, 2024
Many of the windows in the game, especially the ones with scrollbars,
create multiple overlapping windows and then draw them in an odd
order. This results in overdraw, but that was partly compensated for
by the cache that we removed. Now that we clear the whole window in
one go, this causes artifacts as we erase text that was already drawn
in a different window that overlaps the current one. By erasing only
the lines that are touched, we avoid the problem.

Fixes CleverRaven#71994.
@inverimus
Copy link
Contributor

I now get a blank screen when starting the game that then draws the main menu on any keypress. Is that related to this?

@db48x
Copy link
Contributor

db48x commented Feb 28, 2024

Can you give us some more details? What OS? Is the ImGui interface turned on? Can you share a screenshot?

@db48x
Copy link
Contributor

db48x commented Feb 28, 2024

On second thought, a screenshot of a blank screen might not tell us much :) A video might explain it better. Is the main menu displayed normally once you press a key, or does it go away again, or what?

@katemonster33
Copy link
Contributor

I now get a blank screen when starting the game that then draws the main menu on any keypress. Is that related to this?

Possibly, not likely. Open a new issue with all your platform and configuration details please

@IdleSol
Copy link
Contributor Author

IdleSol commented Feb 28, 2024

Separately report ImGui uninitialized errors on Windows (it is vital to include the crash log here, for the stack trace)

#72047
I think it's the same bug. Only in Linux.

Maleclypse pushed a commit that referenced this issue Feb 28, 2024
* clear window line by line, skipping untouched lines
Many of the windows in the game, especially the ones with scrollbars,
create multiple overlapping windows and then draw them in an odd
order. This results in overdraw, but that was partly compensated for
by the cache that we removed. Now that we clear the whole window in
one go, this causes artifacts as we erase text that was already drawn
in a different window that overlaps the current one. By erasing only
the lines that are touched, we avoid the problem.

Fixes #71994.

* format the code to placate astyle

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@IdleSol
Copy link
Contributor Author

IdleSol commented Mar 1, 2024

@db48x @katemonster33

Separately report ImGui uninitialized errors on Windows (it is vital to include the crash log here, for the stack trace)

cdda-windows-tiles-x64-2024-03-01-0629. Fixed game crash. (Probably #72074)

UPD.
arch + portproton: fixed
windows 10 (virtual machine): fixed

@NetSysFire NetSysFire unpinned this issue Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds Info / User Interface Game - player communication, menus, etc. (P2 - High) High priority (for ex. important bugfixes) (S2 - Confirmed) Bug that's been confirmed to exist
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants