Skip to content

Fix ItemInfo struct layout (ImGuiTestItemInfo bitfields)#22

Open
aplavin wants to merge 4 commits into
JuliaImGui:masterfrom
aplavin:iteminfo-bitfields
Open

Fix ItemInfo struct layout (ImGuiTestItemInfo bitfields)#22
aplavin wants to merge 4 commits into
JuliaImGui:masterfrom
aplavin:iteminfo-bitfields

Conversation

@aplavin
Copy link
Copy Markdown
Contributor

@aplavin aplavin commented Jun 3, 2026

te.ItemInfo(...) returned wrong RectFull/flags: ImGuiTestItemInfo was read with the wrong field offsets because the C++ struct packs NavLayer : 1 / Depth : 16 bitfields, making it 104 bytes (the binding assumed 112). Every field from offset 48 on was shifted, so RectFull was read 4 bytes off.

Regenerated against CImGuiPack_jll 0.12.2 (whose cimgui_te.h now carries the bitfields) → correct 104-byte ImGuiTestItemInfo, and the structs that embed it by value (ImGuiTestContext, ImGuiTestInfoTask) get the matching opaque layout. Adds a struct-layout regression test comparing te.ItemInfo(...).RectFull to ig.GetItemRectMin/Max for the same widget.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 28.75%. Comparing base (7bccc5a) to head (5b7595e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   28.22%   28.75%   +0.52%     
==========================================
  Files           5        5              
  Lines         960      960              
==========================================
+ Hits          271      276       +5     
+ Misses        689      684       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread lib/i686-linux-gnu.jl
end

struct ImGuiTestGenericVars
mutable struct ImGuiTestGenericVars
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this and other structs now mutable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's Clang.jl-generated...

Comment thread Project.toml
uuid = "464e2eba-0a11-4ed3-b274-413caa1a1cca"
authors = ["JamesWrigley <james@puiterwijk.org>"]
version = "1.0.3"
version = "1.0.4"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also need to update the changelog entry for the new release.

Comment thread test/runtests.jl
# te.ItemInfo and ig.GetItemRectMin/Max query the same widget, so their
# rects must agree. A mismatch means the ImGuiTestItemInfo binding's field
# offsets disagree with the compiled C++ struct (packed NavLayer:1 / Depth:16).
@test sizeof(lib.ImGuiTestItemInfo) == 104
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not put this in a test, it's way too easy to break in new versions of the test engine.

Comment thread lib/i686-linux-gnu.jl
FrameCount::Cint
DebugName::NTuple{64, Cchar}
Result::ImGuiTestItemInfo
data::NTuple{168, UInt8}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow it seems odd to me that a struct containing bitfield structs as fields itself gets treated as a bitfield struct by Clang 🤔 @Gnimuc, do you know if that's right? It looks like it calculates the offsets correctly and everything, I just don't know if it's actually needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants