GL: re-implement the RSP pipeline on top of magma by snacchus · Pull Request #906 · DragonMinded/libdragon

snacchus · 2026-06-13T11:10:27Z

This PR completely replaces the RSP pipeline of the OpenGL implementation with an improved version that is based on magma. This results in overall greatly improved performance (up to ~4,6x less RSP time required has been measured).

Overview

The rspq overlay rsp_gl_pipeline has been removed and replaced with a magma vertex shader of the same name. Like the old overlay, the shader expects vertices in a fixed format, but flexible layout. Since magma works most efficiently when loading large batches of vertices, the new implementation therefore has to convert all vertex data into internal buffers before handing them off to magma. The data is converted on demand when draw calls are dispatched.

In some cases, whenever GL is able to track changes to vertex layout and vertex data, converted data may be retained and reused across frames. This is the case when using vertex buffer objects and vertex array objects in conjunction. This is consequently the most optimal way to render geometry now, as opposed to display lists with the old implementation.

Because best practices for optimal performance are changing with this re-implementation, drawing models with model64 will not be optimal for now. The performance benefits from the new implementation still outweigh this pessimization, however (see measurements below). An improved version of model64 will follow in a later PR.

Performance comparison

The following measurements were made on a real PAL N64.

This is the baseline, using the old RSP pipeline:

Compare with the new, magma based implementation without further modifications to model64:

And finally, the new implementation using an optimized version of model64:

Breaking changes

The currently bound GL_ELEMENT_ARRAY_BUFFER_ARB is no longer a global state and instead now scoped to the current vertex array object. This now correctly implements the GL specification.
Texture coordinates with 3 or more components don't work correctly in the new implementation. They are effectively treated as 2 components. Depending on demand, this will be fixed in the future.
Tex gen with GL_OBJECT_PLANE and GL_EYE_PLANE is no longer supported by the RSP pipeline. The CPU pipeline will be used instead.

Other changes

The new packed format GL_SHORT_5_6_5_N64 has been added and is available for use in glNormalPointer.
The custom hashtable implementation in obj_map.h has been removed and usages migrated to hashtable_internal.h.

Dragorn421

(this visually passes my triangle tests at https://github.com/Dragorn421/n64homebrew/tree/main/gltest (on ares))

rasky

I took the time of going through the PR. I didn't have time to read it all or rather I don't understand the full architecture. So I just jumped here and there, studied a bit of code irrespective of the context, and just browsed it in the editor leaving comments. I think I might have spotted a few bugs from patterns that looked unconvincing but again, missing context, it's a bit hard for me to make sure a bug is a bug.

rasky · 2026-09-14T22:00:18Z

+    mg_input_assembly_parms_t input_assembly_parms = array_object_get_input_assembly_parms(state->array_object, mode, range);
+    mg_ex_draw(&input_assembly_parms, count, first, mode);
+    mg_draw_end();
 }


If you create the array object already offseted by first, why you need to pass first to mg_ex_draw? Is that a bug? Did you test glDrawArray with first != 0?

Well spotted! That was indeed a bug.

rasky · 2026-09-14T22:13:24Z

    assertf(!state->begin_end_active, "glCallList between glBegin/glEnd is not supported!");

-    rspq_block_t *block = obj_map_get(&state->list_objects, n);
+    rspq_block_t *block = hashtable_lookup(&state->lists, n);


Switching from obj_map to hashtable seems to have removed the NULL check. I think both 0 and TOMBSTONE_KEY are reserved values that should return GL_INVALID_VALUE instead of crashing

rasky · 2026-09-14T22:19:36Z

+    for (array_type_t i = 0; i < ARRAY_COUNT; i++)
+    {
+        parms->arrays[i] = &state->array_object->arrays[i];
+    }


Shouldn't this be filtered by is_array_used? We have a similar filter in data_source.c. It would seem that arrays could easy have NULL pointers otherwise and lead to crashes?

rasky · 2026-09-14T22:24:49Z

+            VTEMP = vlcol:sfract +* vlpos:sfract.xxxxXXXX;
+            VTEMP = vlcol:sfract +* vlpos:sfract.yyyyYYYY;
+            vlcol = vlcol:sfract +* vlpos:sfract.zzzzZZZZ;
+            vlcol = max(vlcol, VZERO);


I think the N*L dot product should be clamped (like we do gl_clamped_dot in the CPU pipeline. Also I'm not sure about how the attenuation is handled. Should it be only applied to vlcol (which is the diffuse term if I understand correctly?)

rasky · 2026-09-14T22:26:29Z

+
+static void gl_rsp_draw_elements(GLenum mode, uint32_t count, const void* indices, GLenum type)
+{
+    assertf(type == GL_UNSIGNED_SHORT, "Index type must be GL_UNSIGNED_SHORT");


We were handling different types before. I guess this is not a huge issue, but maybe it should be documented as a change.

rasky · 2026-09-14T23:06:55Z

+typedef struct {
+    uint32_t offsets[ARRAY_COUNT];
+    uint32_t stride;
+} data_layout_t;


It seems that you're then not using array_type_t to index offsets[], but rather a compact index. This maybe also warrants various comments as it can be tricky to get right.

rasky · 2026-09-14T23:07:49Z

+    const data_layout_t *out_layout;
+    index_bounds_t range;
+    void *out_buffer;
+} array_convert_parms_t;


I could use some documentation of this structure and how the various fields relate.

rasky · 2026-09-14T23:09:09Z

+    uint32_t offset;
+    uint32_t count;
+    GLenum mode;
+} draw_call_parms_t;


Can you also document this structure? I think offset is a byte offset, while count is an element count. This is non-trivial.

rasky · 2026-09-14T23:11:41Z

+#define SCRATCH_MAT_DIFFUSE  0x30
+#define SCRATCH_MAT_AMBIENT  0x40
+#define SCRATCH_MAT_EMISSIVE 0x50
+#define SCRATCH_RGBA_IN      0x60


I guess these offsets are into a scratch area? But we don't have a RSPL include file from Magma that defines the base offset for it?

rasky · 2026-09-14T23:15:58Z

@@ -0,0 +1,573 @@
+include "rsp_magma.inc"


Maybe a top-level comment explaining what this ucode does, how it interacts with the others, etc.

snacchus force-pushed the gl-magma-port branch from 491d66e to 18ad8a7 Compare June 14, 2026 13:30

snacchus marked this pull request as ready for review June 14, 2026 13:41

GL: re-implement the RSP pipeline on top of magma

783e49b

snacchus force-pushed the gl-magma-port branch from 18ad8a7 to 783e49b Compare August 23, 2026 15:02

Dragorn421 reviewed Sep 2, 2026

View reviewed changes

rasky reviewed Sep 14, 2026

View reviewed changes

Navigation Menu

GL: re-implement the RSP pipeline on top of magma - #906

GL: re-implement the RSP pipeline on top of magma#906
snacchus wants to merge 1 commit into
DragonMinded:previewfrom
snacchus:gl-magma-port

snacchus commented Jun 13, 2026 •

edited

Loading

Uh oh!

Dragorn421 left a comment

Uh oh!

rasky left a comment

Uh oh!

rasky Sep 14, 2026

Uh oh!

snacchus Sep 15, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

rasky Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

snacchus commented Jun 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Performance comparison

Breaking changes

Other changes

Uh oh!

Dragorn421 left a comment

Choose a reason for hiding this comment

Uh oh!

rasky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snacchus commented Jun 13, 2026 •

edited

Loading