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

Flickering Bug with SplitChannel, / MergeChannels and DrawList->AddImage #2506

Closed
aiekick opened this issue Apr 20, 2019 · 15 comments
Closed

Comments

@aiekick
Copy link
Contributor

aiekick commented Apr 20, 2019

Version: .70 WIP (16990)
Branch: master

I have a weird flickering bug.

I use SplitChannel, / MergeChannels for have a style applied on a bg of a group

i have a pingpong texture displayed with drawlist->AddImage , so the id of the texture displayed change after each swap.
see in the gif, the flickering is due to the image display. when i mouse over the flickering stop

if i comment drawlist->AddImage, it work, when i uncomment, the next items are flckering.
not sure if its a bug of imgui or from my softs.

2019-04-20_22-22-05

@ocornut
Copy link
Owner

ocornut commented Apr 20, 2019

Hello,
Could you try to make a minimal repro?
Thank you.

@aiekick
Copy link
Contributor Author

aiekick commented Apr 20, 2019

ok, i will try.

@aiekick
Copy link
Contributor Author

aiekick commented Apr 22, 2019

its difficult to do a small repro.

the problem appear only when i split channels, and only with my pingpong system.

each rendering of my texture, i swap the texture id. (back becone front and front become back of what its uploaded to the gpu)

after that i calc and render imgui.

i dont know how, but my texture is used by the next imgui item control. this is why its filickering.

i will inspect the split channel method

In fact, the filckkering stop when i add a text with drawList->AddText

@ocornut
Copy link
Owner

ocornut commented May 18, 2019

@aiekick Any news on this?
The point of a repro is to narrow down the issue. There's probably a bug in the channel splitter but it would help if you tried to made a repro. It doesn't look hard, and I can't be guessing what your code is doing.

EDIT I am pretty confident your double-buffer system has nothing to do with it, as ImDrawList won't see any of that on a given frame.

@aiekick
Copy link
Contributor Author

aiekick commented May 19, 2019

i know what is the goal of that. but i have tried to do a small repro but i not got the bug. i thinck i have a gl context bug since start of NoodlesPlate dev, and maybe some memory leaks somwhere but nout found where (because i see sometime the font texture in place of my textures.
the problem occur when i switch the texture id, but liake said not found a bug with a minimal case. i will search more, maybe is near the texture binding, when i fill one..

@aiekick
Copy link
Contributor Author

aiekick commented Jul 29, 2019

finally succeed to isolate the problem. i have created a repo with a fully workable example : https://github.com/aiekick/ImGui_DrawList_ChannelsSplitter_Bug
use the GLFW Opengl3 sample and try
you were right, my double buffured system is not the problem, but a texture creation before the loop.. its weird

@aiekick
Copy link
Contributor Author

aiekick commented Aug 7, 2019

i found, that if i intialize my texture after your (for the font), it work like a charm.
we see the font texture in each framed group and all texts.

is there a possibility than using a texture number other than 1 for the font (the first texture is the font i thinkc), can cause a bug somewhere ? i know we can have more font so more texture numlber, but the frist font is always 1 no ?, if his initialization in done first of all.

@aiekick
Copy link
Contributor Author

aiekick commented Aug 7, 2019

i have remove my texture creation and,

in ImGui_ImplOpenGL3_CreateFontsTexture()

i have duplicated the line "glGenTextures(1, &g_FontTexture);"

like that i ensure than the first texture number will be 2 instead of 1. the bug is appearing

no texts under ImGui::Images

@aiekick
Copy link
Contributor Author

aiekick commented Aug 7, 2019

i thinck the bug is in ImDrawListSplitter::Merge(ImDrawList* draw_list)

at last if i always do AddDrawCmd instead of UpdateClipRect,=> no bug.
maybe we need to do UpdateTextureID() after UpdateClipRect no ? it resolve the bug too
maybe in some case he can add two drawcalls. not sure if its possible and/or normal :) so maybe we can don a function who group these two's

in my case UpdateClipRect not trigger a drawcall.
but the UpdateTextureID yes :)
can we have UpdateClipRect and UpdateTextureID both triggering a drawcall both ?

in layered imgui,
when i use only ImGui::Image (whti different id than font) and some AddLine, AddRect after => bug
if i use ImGui::Image and ImGui::text => no bug

can't help more i thinck :)

i have modify imgui in my app code, for have the UpdateTextureID after the UpdateClipRect . and there is no more bug now :)

@ocornut
Copy link
Owner

ocornut commented Sep 17, 2019

I would like to get this fixed but it is still extremely tedious to wade through the noise of an entire repo and so many messages, for what should be a 10 lines repro...

@aiekick
Copy link
Contributor Author

aiekick commented Sep 17, 2019

i know but it was not easy to found the bug. im not an expert in dev..
for summary i have added "draw_list->UpdateTextureID();" after the " draw_list->UpdateClipRect();" in ImDrawListSplitter::Merge(ImDrawList* draw_list)

@ocornut
Copy link
Owner

ocornut commented Sep 17, 2019

All I have asked is for a minimal repro (instead of large project) in order to understand the bug.
Here I made it now:

// Confirm hard-coded values 0x01 (atlas) and 0x02 (empty) do what we expect with our OpenGL back-end
{
    ImGui::Text("1:");
    ImGui::SameLine();
    ImGui::Image((ImTextureID)1, ImVec2(50, 50));   // Confirming this is our atlas
    ImGui::SameLine();
    ImGui::Text("2:");
    ImGui::SameLine();
    ImGui::Image((ImTextureID)2, ImVec2(50, 50));   // Confirming this is showing as black
}

// Bug repro
{
    ImDrawList* draw_list = ImGui::GetWindowDrawList();
    draw_list->ChannelsSplit(2);
    draw_list->ChannelsSetCurrent(0);
    ImGui::Text("Hello");
    draw_list->ChannelsSetCurrent(1);
    ImGui::Image((ImTextureID)2, ImVec2(50, 50));
    draw_list->ChannelsMerge();
}

image

ocornut added a commit that referenced this issue Sep 17, 2019
@ocornut
Copy link
Owner

ocornut commented Sep 17, 2019

Now I have the repro I can understand and fix the bug, should be fixed now.
I understand you had found the fix before, but you never let me understand the issue. I cannot apply the fix if I don't understand the issue. This is why the minimal repro is required.

Thanks for reporting!

@ocornut ocornut closed this as completed Sep 17, 2019
@aiekick
Copy link
Contributor Author

aiekick commented Sep 17, 2019

i have created a project here :https://github.com/aiekick/ImGui_DrawList_ChannelsSplitter_Bug ..
it was a full repro of the bug. its easy now after some iterations but it was not before that

@ocornut
Copy link
Owner

ocornut commented Sep 17, 2019

Lots of noise (unnecessary code to understand) in this entire repository, the minimal repro I postes above is 7 lines, it can’t get simpler.

Thanks for finding this!

corentin-plouet added a commit to corentin-plouet/imgui that referenced this issue Sep 22, 2019
Integrated power saving mode in Allegro example.

Fixed the cursor-blinking issue.

It is now working as expected, i.e. only setting a frame rate
requirement of 6fps when visibly blinking.

Also refactored a bit such that ImGui uses a boolean flag instead of
sharing the FrameRateRequirements object with user code.

Renamed FrameRateRequirements -> UserFrameRateRequirements.

Just so it's more explicit.

Minor formatting fixes [noci]

Implemented support for event waiting timeout in Win32.

Simplified the implementation.

This also addresses a design issue regarding how the application
requests a specific frame rate.

Get rid of the minimum frame rate.

Implemented logic to always render at least 3 frames.

Plus a bit of renaming/refactoring.

Implemented the 3-frame update logic for the SDL example.

Added support for 3-frame updates to Allegro example

+ refactoring of SDL/OpenGL3 example
+ refactoring and bug fix of Win32/DX11 example

Added suppor for 3-frame updates to Glfw example.

Rebased imstb_rectpack on stb_rect_pack v1.00.

SliderScalar: Improved assert when using U32 or U64 types with a large v_max value. (ocornut#2765)
+  misc minor stuff.

Demo: PlotLine example displays the average value. (ocornut#2759) + extra comments

Added a mechanism to compact/free the larger allocations of unused windows (buffers are compacted when a window is unused for 60 seconds, as per io.ConfigWindowsMemoryCompactTimer = 60.0f). Note that memory usage has never been reported as a problem, so this is merely a touch of overzealous luxury. (ocornut#2636)

Disable with ConfigWindowsMemoryCompactTimer < 0.0f (ocornut#2636)

TabBar: improved shrinking for large number of tabs to avoid leaving extraneous space on the right side. Individuals tabs are given integer-rounded width and remainder is spread between tabs left-to-right.

TabBar: feed desired width (sum of unclipped tabs width) into layout system to allow for auto-resize. (ocornut#2768)
Before 1.71 tab bars fed the sum of current width which created feedback loops in certain situations. Amend f95c77e.

DragInt, DragFloat, DragScalar: Using (v_min > v_max) allows locking any edit to the value.

ColorEdit: Disable Hue edit when Saturation==0 instead of letting Hue values jump around.

Fixed missing IMGUI_API for IsMouseDragPastThreshold().

Renamed SetMaxTimeBeforeNewFrame -> SetMaxWaitBeforeNextFrame

Compute the proper time to flip the cursor, instead of using 6fps.

Due to the 3-frame logic, this makes the effective cursor frame
rate go from 15fps to 5fps.

Refactored the event waiting implementation in SDL.

Moved the waiting part into the common implementation file.
This makes the platform+renderer binding simpler and not deviating
much from the existing one, which should make merging/integration
easier, especially for people maintaining their own copies.

Renamed variable to match the method name.

Refactored Win32 waiting code into common platform file

+ fix typo in SDL example

Fixed build

Avoid wasting frames by adding a small margin for the cursor.

Fixed regression

Win32 example: don't render 3 frames on timeouts

+ some refactoring

Refactored+fixed the Allegro5 implementation

Refactored+fixed Glfw example

Refactored+fixed SDL example

Refactoring (renaming)

Some final cleanup/refactoring; make the diff better

Refactored: no need to conditionally (not) poll after wait.

Keep waiting when the window is hidden (or minimized).

Implemented for Allegro, Glfw and SDL.
(does not seem to work with Allegro on Linux)

Implemented blocking when minimized/hidden for Win32

Minor formatting/doc changes.

Knocked off the 3 TODO items implemented by this PR.

Added missing Glfw callback (mouse pos)

This is just to record the event. The mouse pos is still handled as
before.

Small optimization: added shortcut test when feature disabled

This avoids paying the library/syscall cost when not needed.

Backends: OpenGL3: Tweaked initialization code allow application calling ImGui_ImplOpenGL3_CreateFontsTexture() before ImGui_ImplOpenGL3_NewFrame() if for some reason they wanted.

Fix DragScalar for unsigned types (ocornut#2780)
decreasing the value was broken on arm64

Nav, Scrolling: Added support for Home/End key. (ocornut#787)

Columns: Separator: Fixed a bug where non-visible separators within columns would alter the next row position differently than visible ones.
Fixed rounding issues also leading to change of ScrollMax depending on visible items (in particular negative coordinate would be rounded differently)

Fix signed types warning in pasteboard handler (ocornut#2786)

Examples: SDL/GLFW + OpenGL3: Fixes for Makefile (ocornut#2774)

- append CXXFLAGS instead of overwriting them
- add glad.c build rule

BeginTabItem: Fixed case where right-most tab would create an extraneous draw calls (probably related to other tab fitting code in 1.73 wip)

Remove trailing spaces (grep for ' \r?$' in visual studio)

Internal: Offset STB_TEXTURE_K_ defines to remove that change from ocornut#2541 + sponsors update.

Font: implement a way to draw narrow ellipsis without relying on hardcoded 1 pixel dots. (ocornut#2775)

This changeset implements several pieces of the puzzle that add up to a narrow ellipsis rendering.

`ImFontConfig` and `ImFont` received `ImWchar EllipsisCodePoint = -1;` field. User may configure `ImFontConfig::EllipsisCodePoint` a unicode codepoint that will be used for rendering narrow ellipsis. Not setting this field will automatically detect a suitable character or fall back to rendering 3 dots with minimal spacing between them. Autodetection prefers codepoint 0x2026 (narrow ellipsis) and falls back to 0x0085 (NEXT LINE) when missing. Wikipedia indicates that codepoint 0x0085 was used as ellipsis in some older windows fonts. So does default Dear ImGui font. When user is merging fonts - first configured and present ellipsis codepoint will be used, ellipsis characters from subsequently merged fonts will be ignored.

Rendering a narrow ellipsis is surprisingly not straightforward task. There are cases when ellipsis is bigger than the last visible character therefore `RenderTextEllipsis()` has to hide last two characters. In a subset of those cases ellipsis is as big as last visible character + space before it. `RenderTextEllipsis()` tries to work around this case by taking free space between glyph edges into account. Code responsible for this functionality is within `if (text_end_ellipsis != text_end_full) { ... }`.

There are cases when font does not have ellipsis character defined. In this case RenderTextEllipsis() falls back to rendering ellipsis as 3 dots, but with reduced spacing between them. 1 pixel space is used in all cases. This results in a somewhat wider ellipsis, but avoids issues where spaces between dots are uneven (visible in larger/monospace fonts) or squish dots way too much (visible in default font where dot is essentially a pixel). This fallback method obsoleted `RenderPixelEllipsis()` and this function was removed. Note that fallback ellipsis will always be somewhat wider than it could be, however it will fit in visually into every font used unlike what `RenderPixelEllipsis()` produced.

Font: Narrow ellipsis: various minor stylistic tweaks (ocornut#2775)

Font: Narrow ellipsis: once we know an ellipsis is going to be drawn, we can claim the space between pos_max.x and ellipsis_max.x which gives us enough extra space to not requires the further (and otherwise valid) optimizations. Gets us vastly simplified code, yay. (ocornut#2775)

Style: Allow style.WindowMenuButtonPosition to be set to ImGuiDir_None to hide the collapse button. (ocornut#2634, ocornut#2639)
+ Fix ocornut#2775

ImDrawListSplitter: fixed an issue merging channels if the last submitted draw command used a different texture. (ocornut#2506)

Fixed unused static function warning for some compilers. (ocornut#2793)

TreeNode: Added ImGuiTreeNodeFlags_SpanAvailWidth and ImGuiTreeNodeFlags_SpanFullWidth flags (ocornut#2451, ocornut#2438, ocornut#1897)
Added demo bits.

Warning fix.

ColorPicker / ColorEdit: restore Hue when zeroing Saturation. (ocornut#2722, ocornut#2770)
Issue is fixed by storing last active color picker color and last hue value when active color picker takes rgb as input. Then if current color picker color matches last active color - hue value will be restored. IDs are not used because ColorEdit4() and ColorWidget4() may call each other in hard-to-predict ways and they both push their own IDs on to the stack. We need hue restoration to happen in entire stack of these widgets if topmost widget used hue restoration. Since these widgets operate on exact same color value - color was chosen as a factor deciding which widgets should restore hue.

ColorPicker / ColorEdit: restore Hue when zeroing Saturation. (ocornut#2722, ocornut#2770) - changelog, fixed uninitialized variables, tweaks, renaming.

Fixed mouse event forwarding in macos example (ocornut#2710, ocornut#1961)

Readme, Wiki: Image loading examples.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants