mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-17 08:38:30 +09:00
[Fix] (Pipe): carry the class a vertex-attribute default was written through, and stop the applier's lossy write from being observable
- set_vertex_attrib_defaults hard-coded MGPAttribValue::ValueClass to 0 for every attribute and always sent the FLOAT view's four words. A CurrentVertexAttributeValue is one value in three views and GLContext converts numerically between them, so those bytes cannot reproduce the frontend value: glVertexAttrib4f(loc, 1.5f, ..) leaves 1 in intValue and 0x3FC00000 in floatValue, and every glVertexAttribI4i/ui default was wrong too - GLContext now records which view each glVertexAttrib* write filled directly (GetCurrentVertexAttributeClass, push-only) and the payload carries that class and THAT class's own words. It is kept beside the value rather than inside it because CurrentVertexAttributeValue is mirrored into PipeInputs and compared there by a memcmp whose size assertion lives in a file this package does not own - MGPipeFillAttribValue is the flattening, in one named place, so TrackerAttribPayload can pin it: the old defect turns three of its four cases red - the applier (package A's) still memcpys the four words into all three views regardless of ValueClass, so the emitter now CHECKS: it compares the mirror the applier wrote against the frontend's value and, when they differ, copies the field itself and says so once. That closes the window the old code left wrong - a glVertexAttrib* write followed by a verb whose class does not read the field, where the residual fill does not run for it - and it stops repairing by itself the day A's applier honours the class - MGPipeVertexAttribDefaultRepairCount() makes that repair observable to a test without reading storage the fill table forbids that verb to read - the emission gate now goes through MGPipeSubsystemForDirty, the one bit-to-subsystem map, instead of a second copy of it written out by hand at the validate point; five static_asserts tie that map to the field-emitter map it has to agree with - the staging mirror is advanced only by the branch that sent dynamic bytes, with the invariant it used to rely on (BumpVersions moves both counters, RenderState.h) asserted here rather than assumed of another package's file - TrackerShippedEmitter drives MGPipeValidateForVerb itself and reads the real singletons back, so the blend-toggle and viewport shapes are pinned on the shipped emitter and not only on the unit tests' local re-implementation; the fixtures reset the applier, the cache and the tracker together, which is the only consistent state of the three - comments: the derivation probe is a one-field sample, the residual block's trip wire is half a tautology until package A's c1 lands, and the widened counter cannot see a change of exactly 65536 - all three recorded where the code is, not only in a review
This commit is contained in:
@@ -149,6 +149,13 @@ namespace MobileGL::MG_Pipe {
|
||||
// (ARCHITECTURE.md 5.2: MG_State is not changed for this). A decrease is a wrap and adds
|
||||
// 65536. A wrap is harmless locally - one extra re-push, never a missed one - which is
|
||||
// exactly what TrackerTest.WrapAroundRePushesButNeverMisses pins.
|
||||
//
|
||||
// THE ONE CASE IT CANNOT SEE, stated because "never a missed push" is otherwise stronger
|
||||
// than what is true: the wrap test is `now < m_last`, so a counter that advances by
|
||||
// EXACTLY 65536 (or a multiple) between two walks reads as unchanged. That needs 65536
|
||||
// render-state mutations inside one verb boundary, and it is pre-existing in class -
|
||||
// both backends already compare raw Uint16 versions the same way - so P2 records it
|
||||
// rather than widening MG_State's counters, which ARCHITECTURE.md 5.2 rules out.
|
||||
class MGPipeWidenedCounter {
|
||||
public:
|
||||
Uint64 Observe(Uint16 now) {
|
||||
@@ -392,6 +399,33 @@ namespace MobileGL::MG_Pipe {
|
||||
Uint64 m_walks[kMGPipeVerbClassCount]{};
|
||||
};
|
||||
|
||||
// ONE attribute default, flattened onto the wire (P2 brief D10). A named function rather
|
||||
// than four lines inside the emitter because this flattening is the whole correctness
|
||||
// question of set_vertex_attrib_defaults: a CurrentVertexAttributeValue is one value in
|
||||
// three views and GLContext converts NUMERICALLY between them, so four words alone are
|
||||
// not the value - glVertexAttrib4f(loc, 1.5f, ...) leaves 1 in intValue and 0x3FC00000 in
|
||||
// floatValue. MGPAttribValue::ValueClass is what makes the four words readable again, and
|
||||
// TrackerAttribPayload pins that here instead of leaving it to the emitter's shape.
|
||||
inline void MGPipeFillAttribValue(Uint32 location,
|
||||
const MG_State::GLState::CurrentVertexAttributeValue& value,
|
||||
Uint32 writtenClass, MGPAttribValue& out) {
|
||||
out = MGPAttribValue{};
|
||||
out.Location = location;
|
||||
out.ValueClass = static_cast<Uint8>(writtenClass);
|
||||
static_assert(sizeof(out.Data) == sizeof(value.floatValue), "MGPAttribValue::Data is four words");
|
||||
switch (writtenClass) {
|
||||
case MG_State::GLState::kVertexAttribValueClassInt:
|
||||
std::memcpy(out.Data, value.intValue.data(), sizeof(out.Data));
|
||||
break;
|
||||
case MG_State::GLState::kVertexAttribValueClassUint:
|
||||
std::memcpy(out.Data, value.uintValue.data(), sizeof(out.Data));
|
||||
break;
|
||||
default:
|
||||
std::memcpy(out.Data, value.floatValue.data(), sizeof(out.Data));
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// The monolith's one tracker. Under split there is one per client context; the context
|
||||
// identity check inside Update is what makes the single instance safe today.
|
||||
inline MGPipeTracker& MGPipeTrackerInstance() {
|
||||
|
||||
Reference in New Issue
Block a user