Skip to content

Fix 8000 vert level editor crash #445

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.

Sign up for GitHub

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

Draft
GooberRF wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Fix 8000 vert level editor crash #445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ Version 1.5.0 (TBD): Not yet released
### Major features

### Minor features, changes, and enhancements
[@GooberRF](https://github.com/GooberRF)
- Raise level editor per-room and per-mesh render vertex limit from 8000 to 32768

### Bug fixes
[@GooberRF](https://github.com/GooberRF)
- Fix level editor crashing without an error message when drawing a room or mesh containing more than 8000 vertices

Version 1.4.0 (Lupin): Released Aug-25-2026
--------------------------------
Expand Down
85 changes: 85 additions & 0 deletions editor_patch/graphics.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <d3d8.h>
#include <algorithm>
#include <cmath>
#include <initializer_list>
#include "vtypes.h"
#include "alpine_obj.h"

Expand All @@ -33,6 +34,33 @@ static constexpr int max_detail_rooms = 8192;
static void* detail_room_list[max_detail_rooms];
static int detail_room_count;

// RED transforms each room into parallel per-vertex arrays hardcoded to 8000 entries, and the
// per-vertex loops have no bounds check: a bigger room runs the 20-byte screen vert array off the
// top of FUN_00502040's stack frame and smashes its callers. 32768 is the int16 index ceiling.
static constexpr int max_room_render_verts = 0x8000;
static u8 room_vert_flags[max_room_render_verts]; // stack, frame+0x98c
alignas(16) static u8 room_screen_verts[max_room_render_verts * 20]; // stack, frame+0x51cc
alignas(16) static u8 room_xformed_verts[max_room_render_verts * 12]; // static, 0x01a85500
static u8 shared_vert_flags[max_room_render_verts]; // static, 0x01ab4354

// Same pattern in the v3d mesh LOD renderers (FUN_00505c60 / FUN_00506830), which also share
// room_xformed_verts and shared_vert_flags. Mesh chunk vertex counts are u16.
alignas(16) static u8 mesh_verts_a[max_room_render_verts * 12]; // static, 0x01a520b0
static u8 mesh_vert_flags[max_room_render_verts]; // static, 0x01a697b0
alignas(16) static u8 mesh_verts_d[max_room_render_verts * 12]; // static, 0x01a6de00
alignas(16) static u8 mesh_verts_c[max_room_render_verts * 12]; // static, 0x01a9cc00
static u8 mesh_vert_rgb[max_room_render_verts * 3]; // static, 0x01ab6294

// Repoint references to a relocated static array. Sites are operand addresses; the field offset
// the instruction uses is recovered from its current value rather than repeated per site.
static void repoint_array_refs(std::initializer_list<uintptr_t> sites, uintptr_t old_base,
const u8* new_base)
{
for (uintptr_t operand : sites) {
write_mem_ptr(operand, new_base + (addr_as_ref<u32>(operand) - old_base));
}
}

namespace red
{
struct GrScreen
Expand Down Expand Up @@ -513,6 +541,63 @@ void ApplyGraphicsPatches()
write_mem_ptr(0x0049c034, editor_geo_vertex_chain);
write_mem_ptr(0x0049c226, editor_geo_vertex_chain);

// Point the level renderer's per-vertex loops at the relocated arrays (FUN_00502040). Four
// sites are a byte shorter as an absolute form, so they pass an end address to be padded.
using namespace asm_regs;
AsmWriter(0x005021f4, 0x005021fb).lea(edi, room_vert_flags);
AsmWriter(0x005022c1).cmp_byte(*(ecx * 1 + room_vert_flags), 0);
AsmWriter(0x005022c9).lea(ecx, *(ecx * 1 + room_vert_flags));
AsmWriter(0x005024cf).cmp_byte(*(ecx * 1 + room_vert_flags), 0);
AsmWriter(0x005024d7).lea(ecx, *(ecx * 1 + room_vert_flags));
AsmWriter(0x00502785).mov(al, *(ecx * 1 + room_vert_flags));
AsmWriter(0x005028ea).mov(cl, *(eax * 1 + room_vert_flags));
AsmWriter(0x00502c8b, 0x00502c92).lea(edi, room_vert_flags);
AsmWriter(0x00502ccf).mov_byte(*(edi * 1 + room_vert_flags), 0);
AsmWriter(0x00502d2a).mov(dl, *(ecx * 1 + room_vert_flags));
AsmWriter(0x00502dc5).mov(dl, *(ecx * 1 + room_vert_flags));
AsmWriter(0x0050277a, 0x00502781).lea(edi, room_screen_verts + 0x10);
AsmWriter(0x005028e3, 0x005028ea).lea(esi, room_screen_verts + 4);
AsmWriter(0x00502d43).lea(edi, *(edx * 4 + room_screen_verts));
AsmWriter(0x00502d4a).mov(dl, *(edx * 4 + room_screen_verts + 0x11));
AsmWriter(0x00502dda).lea(edi, *(edx * 4 + room_screen_verts));
AsmWriter(0x00502de1).mov(edx, *(edx * 4 + room_screen_verts));
AsmWriter(0x00503912).lea(esi, *(ecx * 4 + room_screen_verts));
AsmWriter(0x00503919).mov(cl, *(ecx * 4 + room_screen_verts + 0x11));
AsmWriter(0x005039e8).mov(al, *(edx * 4 + room_screen_verts + 0x11));
AsmWriter(0x00503c4d).mov(ecx, *(edx * 4 + room_screen_verts));
AsmWriter(0x00503c54).lea(eax, *(edx * 4 + room_screen_verts));

// Statics, shared between the level and mesh renderers
repoint_array_refs({0x0050275b, 0x00502f35, 0x005039c1, 0x0050588d, 0x005060d5, 0x005063fa,
0x00506bdc, 0x00507462},
0x01a85500, room_xformed_verts);
repoint_array_refs({0x00502d55, 0x00502ead, 0x00502ebe, 0x00502f49, 0x00505f84, 0x00505f8c,
0x005060ae, 0x005060cd, 0x00506187, 0x005062f3, 0x00506305, 0x00506317,
0x00506332, 0x00506338, 0x00506341, 0x00506378, 0x0050637e, 0x00506387,
0x00506412, 0x00506c1b, 0x00506c27, 0x00506c3f, 0x00507073, 0x00507090,
0x00507164, 0x005072d4, 0x005072e6, 0x005072f8, 0x00507317, 0x00507320,
0x00507326, 0x005073e1, 0x005073ea, 0x005073f0, 0x0050747a},
0x01ab4354, shared_vert_flags);
repoint_array_refs({0x0050582d, 0x00505f54, 0x00506c6e, 0x00506cf7, 0x00506d09, 0x00506d0f,
0x00506ed9, 0x00506edf, 0x00506eee, 0x00506ef4, 0x00506f41, 0x00506f4d,
0x00506f53, 0x00506f5f, 0x00506f71, 0x00506fb0, 0x00506fbc, 0x00506fc2,
0x00506fd2, 0x00506fde, 0x00507000, 0x00507026, 0x00507048, 0x0050704e,
0x00507063, 0x005070a1, 0x005070b3, 0x005070d3, 0x005070ed, 0x00507105,
0x0050714d, 0x005071a5, 0x005071b1, 0x005071cd},
0x01a520b0, mesh_verts_a);
repoint_array_refs({0x00505fa6, 0x005060f5, 0x00506c68, 0x005070c7},
0x01a697b0, mesh_vert_flags);
repoint_array_refs({0x005058bd, 0x00506bf0, 0x00506c21, 0x00506f95, 0x00507353, 0x00507363,
0x00507385, 0x00507395, 0x005073b5},
0x01a6de00, mesh_verts_d);
repoint_array_refs({0x0050585d, 0x00506be8, 0x00506cff, 0x00506d15, 0x00506f03, 0x00506f09,
0x00506f1f, 0x00506f25, 0x00506f65, 0x00506f77, 0x00506f87, 0x00506f9e,
0x00507170, 0x0050718e},
0x01a9cc00, mesh_verts_c);
repoint_array_refs({0x00505f4f, 0x00505f63, 0x00505ffc, 0x00506444, 0x00506bd1, 0x00506cc3,
0x005074b1},
0x01ab6294, mesh_vert_rgb);

// Restore render state after D3D device Reset()
gr_d3d_device_reset_state_recovery.install();
}
143 changes: 120 additions & 23 deletions patch_common/include/patch_common/AsmWriter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,27 +59,84 @@ struct AsmReg8 : public AsmReg
struct AsmRegMem
{
bool memory;
std::optional<AsmReg> reg_opt;
std::optional<AsmReg> reg_opt; // base register
int32_t displacement;
std::optional<AsmReg> index_opt; // scaled index register, if any
uint8_t scale; // 1, 2, 4 or 8; only meaningful with index_opt

AsmRegMem(bool memory, std::optional<AsmReg> reg_opt, int32_t displacement = 0) :
memory(memory), reg_opt(reg_opt), displacement(displacement)
AsmRegMem(bool memory, std::optional<AsmReg> reg_opt, int32_t displacement = 0,
std::optional<AsmReg> index_opt = {}, uint8_t scale = 1) :
memory(memory), reg_opt(reg_opt), displacement(displacement), index_opt(index_opt), scale(scale)
{}

AsmRegMem(AsmReg reg) :
memory(false), reg_opt({reg}), displacement(0)
memory(false), reg_opt({reg}), displacement(0), scale(1)
{}

explicit AsmRegMem(uint32_t addr) :
memory(true), displacement(static_cast<int32_t>(addr))
memory(true), displacement(static_cast<int32_t>(addr)), scale(1)
{}

template<typename T>
AsmRegMem(T* ptr) :
memory(true), displacement(reinterpret_cast<int32_t>(ptr))
memory(true), displacement(reinterpret_cast<int32_t>(ptr)), scale(1)
{}
};

// Accumulates base/index/scale/displacement so `*(ebx + edx * 4 + 0x10)` can be written directly.
struct AsmMemExpr
{
std::optional<AsmReg> base;
std::optional<AsmReg> index;
uint8_t scale = 1;
int32_t displacement = 0;
};

// Validating wrapper: the consteval constructor turns an unencodable scale into a compile error
// instead of a silent encoding of scale 1, while `edx * 4` still reads naturally.
struct AsmScale
{
uint8_t value;

consteval AsmScale(int scale) : value(static_cast<uint8_t>(scale))
{
if (scale != 1 && scale != 2 && scale != 4 && scale != 8) {
throw "SIB scale must be 1, 2, 4 or 8";
}
}
};

inline AsmMemExpr operator*(AsmReg32 index, AsmScale scale)
{
return {{}, {index}, scale.value, 0};
}

inline AsmMemExpr operator+(AsmReg32 base, AsmMemExpr e)
{
assert(!e.base); // `eax + (ebx + ecx * 4)` would otherwise silently drop one of the bases
e.base = {base};
return e;
}

inline AsmMemExpr operator+(AsmMemExpr e, int n)
{
e.displacement += n;
return e;
}

inline AsmMemExpr operator-(AsmMemExpr e, int n)
{
e.displacement -= n;
return e;
}

template<typename T>
inline AsmMemExpr operator+(AsmMemExpr e, T* ptr)
{
e.displacement += reinterpret_cast<int32_t>(ptr);
return e;
}

inline AsmRegMem operator*(AsmReg32 reg)
{
return {true, {reg}, 0};
Expand All @@ -90,6 +147,11 @@ inline AsmRegMem operator*(std::pair<AsmReg32, int> p)
return {true, {p.first}, p.second};
}

inline AsmRegMem operator*(AsmMemExpr e)
{
return {true, e.base, e.displacement, e.index, e.scale};
}

namespace asm_regs
{

Expand Down Expand Up @@ -384,6 +446,31 @@ class AsmWriter
return *this;
}

AsmWriter& mov(const AsmReg8& dst_reg, const AsmRegMem& src_rm)
{
write<u8>(0x8A); // Opcode
write_mod_rm(src_rm, dst_reg);
return *this;
}

// Byte-operand immediate forms. The AsmRegMem/int8_t overloads above are the 0x83 sign-extended
// encodings, which operate on a dword, so the 8 bit operand size needs its own name.
AsmWriter& mov_byte(const AsmRegMem& dst_rm, int8_t imm)
{
write<u8>(0xC6); // Opcode
write_mod_rm(dst_rm, 0);
write<i8>(imm);
return *this;
}

AsmWriter& cmp_byte(const AsmRegMem& dst_rm, int8_t imm)
{
write<u8>(0x80); // Opcode
write_mod_rm(dst_rm, 7);
write<i8>(imm);
return *this;
}

AsmWriter& lea(const AsmReg32& dst_reg, const AsmRegMem& src_rm)
{
write<u8>(0x8D);
Expand Down Expand Up @@ -555,34 +642,44 @@ class AsmWriter

void write_mod_rm(const AsmRegMem& rm, uint8_t reg_field)
{
uint8_t mod_field = 0;
if (!rm.memory)
mod_field = 3;
else if (rm.displacement == 0 || !rm.reg_opt)
constexpr uint8_t esp_num = 4; // rm 100b selects a SIB byte, so esp needs one to be a base
constexpr uint8_t ebp_num = 5; // rm 101b with mod 00b means disp32, so ebp needs a disp8

if (!rm.memory) {
write<u8>((3 << 6) | (reg_field << 3) | rm.reg_opt.value().reg_num);
return;
}

bool has_base = rm.reg_opt.has_value();
bool has_index = rm.index_opt.has_value();
assert(!has_index || rm.index_opt.value().reg_num != esp_num); // esp cannot be an index
assert(rm.scale == 1 || rm.scale == 2 || rm.scale == 4 || rm.scale == 8);
bool need_sib = has_index || (has_base && rm.reg_opt.value().reg_num == esp_num);

uint8_t mod_field;
if (!has_base)
mod_field = 0;
else if (rm.displacement == 0 && rm.reg_opt.value().reg_num != ebp_num)
mod_field = 0;
else if (abs(rm.displacement) < 128)
mod_field = 1;
else
mod_field = 2;

uint8_t rm_field = rm.reg_opt ? rm.reg_opt.value().reg_num : 5;
uint8_t mod_reg_rm_byte = (mod_field << 6) | (reg_field << 3) | rm_field;
write<u8>(mod_reg_rm_byte); // 0xD

// TODO: full SIB support?
if (rm_field == 4 && mod_field != 3) {
uint8_t scale_field = 0;
uint8_t index_field = 4; // no index
uint8_t base_field = 4; // esp
uint8_t sib_byte = (scale_field << 6) | (index_field << 3) | base_field;
write<u8>(sib_byte);
uint8_t rm_field = need_sib ? 4 : (has_base ? rm.reg_opt.value().reg_num : 5);
write<u8>((mod_field << 6) | (reg_field << 3) | rm_field);

if (need_sib) {
auto scale_field = static_cast<uint8_t>(rm.scale == 8 ? 3 : rm.scale == 4 ? 2 : rm.scale == 2 ? 1 : 0);
auto index_field = static_cast<uint8_t>(has_index ? rm.index_opt.value().reg_num : 4); // 100b = none
auto base_field = static_cast<uint8_t>(has_base ? rm.reg_opt.value().reg_num : 5); // 101b = none
write<u8>((scale_field << 6) | (index_field << 3) | base_field);
}

if (mod_field == 1)
write<i8>(rm.displacement);
else if (mod_field == 2 || !rm.reg_opt) {
else if (mod_field == 2 || !has_base)
write<i32>(rm.displacement);
}
}

void write_mod_rm(const AsmRegMem& rm, const AsmReg& reg)
Expand Down
Loading