Skip to content

macOS: render entity models via VBO, not empty display lists - #7

Merged
j92580498-max merged 1 commit into
mainfrom
devin/1778792868-macos-player-preview
May 15, 2026
Merged

macOS: render entity models via VBO, not empty display lists#7
j92580498-max merged 1 commit into
mainfrom
devin/1778792868-macos-player-preview

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Fixes the last reported bug from the previous round: the Armor screen's player preview was empty. Same family of bug as the chunk-VBO regression in baa7155 — a USE_VBO/OPENGL_ES mismatch.

ModelPart::draw() (used for the player, every mob, item-in-hand, etc.) was branching on #ifdef OPENGL_ES:

void ModelPart::draw() {
#ifdef OPENGL_ES
    drawArrayVT_NoState(vboId, cubes.size() * 2 * 3 * 6, 24);
#else
    glCallList(list);
#endif
}

…and compile() symmetrically wrapped the tesselator upload in glNewList(list, GL_COMPILE) … glEndList() on the #ifndef OPENGL_ES path.

On macOS desktop GL 2.1, gles.h defines USE_VBO but does not define OPENGL_ES. Under USE_VBO, Tesselator::end(true, vboId) only uploads geometry into the VBO and never issues any glDrawArrays:

// Tesselator.cpp
glBindBuffer2(GL_ARRAY_BUFFER, bufferId);
glBufferData2(GL_ARRAY_BUFFER, bytes, _varray, access);  // upload
#ifndef USE_VBO
    // … glDrawArrays() lives here, skipped under USE_VBO
#endif

So on macOS, compile() was wrapping a pure VBO upload inside a display list and recording zero draw calls. draw() then called glCallList(list) on that empty list. Result: the model parts were uploaded to GPU memory, but nothing ever drew them — the player preview pane stayed blank, and the same thing would happen for any entity model rendered through ModelPart::draw() (other mobs, third-person player, etc.).

Fix is to gate both halves on USE_VBO instead of OPENGL_ES so each platform takes the matching half of compile + draw:

  • Win32 (USE_VBO undef, OPENGL_ES undef): display-list path. Tesselator::end issues a real glDrawArrays (the #ifndef USE_VBO block), the display list captures it, glCallList replays it. Behavior unchanged.
  • iOS / Android / Raspberry Pi (OPENGL_ESUSE_VBO): VBO path. glNewList/glEndList were already skipped under !defined(OPENGL_ES); they stay skipped under !defined(USE_VBO). Behavior unchanged.
  • macOS desktop (USE_VBO, no OPENGL_ES): VBO path now. compile() just uploads to the VBO, draw() calls drawArrayVT_NoState(vboId, …). Fixed.

Review & Testing Checklist for Human

Yellow risk. The change is small and platform-gated, but it touches every entity model render on every platform — please verify nothing regressed elsewhere:

  • macOS — primary fix. Open Options → Armor on the merged build. The right-side preview pane should now show the player model (head/body/arms/legs animating slowly).
  • macOS — every other mob renders. Step out of the cave / build a world, spawn or find a pig / cow / chicken / sheep / zombie / skeleton — make sure mobs render with their geometry (not just shadow/name) and that nothing visually regressed (e.g. flipped/inside-out, wrong texture).
  • macOS — third-person view of the player. F5 (or wherever 3rd-person toggle lives in this port) — the player model itself should render.
  • Item-in-hand rendering still works in first-person view (the swinging arm + held item — that path also goes through ModelPart for some models).
  • (If you can) smoke-check the Win32 build that nothing in this file's display-list path regressed. The condition flipped name but not effective behavior on Win32 — but worth eyeballing.

Notes

  • This builds on top of PR macOS: load lang/ + sound/aac/, dress up Set username dialog #6 (merged) which fixed lang/sound/dialog issues. That PR's CI smoke test already confirmed the .app boots, loads all 89 sound clips, and shows the main menu correctly.
  • I did not extend the CI smoke test to navigate into the Armor screen in this PR — that needs AppleScript / cliclick automation against an unsigned OpenGL app on macOS GH runners, which is finicky and worth its own PR. For now, the smoke test will still just confirm the .app launches on macos-14 / macos-15.
  • The leftover for (int i = 0; i < 6; i++) outer loop in compile() (the inner one shadows i) is an unrelated old bug — it makes the VBO 6× larger than necessary, but draw() only reads cubes.size() * 36 vertices so the visible geometry is correct. Leaving it alone to keep this PR scoped.

Link to Devin session: https://app.devin.ai/sessions/5a7050a439284734a4b5702cd21fd2b7
Requested by: @j92580498-max

ArmorScreen showed the right-side player preview as an empty pane
because ModelPart::draw() was guarded on OPENGL_ES instead of
USE_VBO, the same shape of bug that baa7155 fixed for chunk VBOs.

On the macOS desktop GL 2.1 build, gles.h defines USE_VBO but does
NOT define OPENGL_ES. Under USE_VBO, Tesselator::end(true, vboId)
only uploads geometry to the VBO and never issues a draw call. The
old compile() wrapped that upload in glNewList / glEndList, so the
display list captured zero draw commands. draw() then called
glCallList on an empty list and nothing rendered: head, body, arms,
legs, all gone — every place an entity model is drawn (player
preview, third-person view, other mobs).

Gate the glNewList/glEndList pair on !USE_VBO and gate the
drawArrayVT_NoState path on USE_VBO so each platform takes the
matching half of compile and draw:
- Win32 (USE_VBO undef, OPENGL_ES undef): display-list path,
  Tesselator::end issues the draw, captured into glNewList. Same
  as before.
- iOS/Android/RPi (OPENGL_ES => USE_VBO): VBO path, unchanged.
- macOS desktop (USE_VBO, no OPENGL_ES): VBO path, FIXED — no more
  empty display list.
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@j92580498-max
j92580498-max merged commit 8aeb481 into main May 15, 2026
2 checks passed
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.

1 participant