mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-07 18:13:23 +09:00
[MG_Backend] (DirectGLES): the monolith client-array snapshot compiles in every flavor
The indexed and indirect client-memory upload added by 62bfe461 landed inside
`#if MOBILEGL_BUILD_DISAGGREGATED`, and that guard was wrong in the one way a
guard can be wrong without a test saying so: the code exists FOR the monolith
arm - every entry point in the family early-returns when `MG_Config::Transport
!= Monolith` - and the monolith arm is what the pull, verify and push builds
run, not a split-arm fallback. Only the split lane was building the change, so
only the split lane could compile it.
Both CI jobs that build without MOBILEGL_BUILD_DISAGGREGATED were red on the
same twenty diagnostics: `build explicit pull runtime control` (the pull
library retrace's monolith control measures) and `build-linux-verify`
(MOBILEGL_PIPE_VERIFY=ON). DirectGLES.cpp:8437/8455/8521/8537 are the two
indirect executors and :9225/9251/9623/9637/9675/9700 the indexed family,
every one of them in an entry point that compiles in all four flavors, calling
names the pull build had no declaration for:
error: no member named 'SyncClientSideVertexArraysForIndirectFetch' in
namespace 'MobileGL::MG_Backend::DirectGLES::VertexArrayImpl'
... and SyncClientSideVertexArraysForIndexedFetch, likewise.
The block moves out of the guard rather than being duplicated per flavor, so
the split build's preprocessed text and behaviour are unchanged: the three
wrappers keep their `Transport != Monolith` early return, and in the
transport-free flavors that comparison folds to a compile-time false
(Config.h:566) exactly as it does in every other monolith-only body in this
file.
What the guard also covered, and moves with it:
* ClientFetchPlan.h - no longer behind `MOBILEGL_BUILD_DISAGGREGATED &&
MOBILEGL_PIPE_PUSH`. The plan IS the monolith arm's arithmetic (the file's
own header says so: "the monolith arm asks it to size the per-draw upload"),
so a pull build that cannot see it cannot answer the question.
* BackendVertexArrayObject::SyncClientSideAttributesForDraw - declaration and
definition, the callee of SyncClientSideVertexArraysForFetch.
* The `#include <MG_Impl/Pipe/ClientFetchPlan.h>` in Managers.h, which had
been inside the same `#if MOBILEGL_PIPE_PUSH` as the two P3a/P4a includes
it was appended to.
G1: the three wrappers and the byte reader take internal linkage (`static`),
the spelling the sibling SyncClientSideVertexArraysForDrawArrays already uses
at DirectGLES.cpp:2422 for exactly this reason. Verified on the pull build
(Release/INFO, LTO off, library target only, the monolith-symbol-report job's
own flags): 0 added / 0 removed / 0 resized / 0 renamed, .text +0, and every
one of the new names is LOCAL (`t`/`b`) with 0 of them in the dynamic table.
Had they kept external linkage they would have been 0 in the pull symbol delta
anyway (CXX_VISIBILITY_PRESET hidden) and 4 in a Debug build's dynamic table,
which is the configuration CMakeLists.txt:629-641 switches to `default`.
Verified: build-pull rc=0, build-push rc=0, build-verify
(MOBILEGL_PIPE_VERIFY=ON, DISAGGREGATED=OFF) rc=0, build-split rc=0; split
regressions green under both MOBILEGL_TRANSPORT=monolith and =inproc for
ClientVertexArrayScenario (18/18 each) and DrawRangeElements|MultiDraw (119/119
each), so the client-array behaviour 62bfe461..cb64b62b repaired is not
regressed.
This commit is contained in:
@@ -2421,9 +2421,22 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// `GetBoundVertexArray@DrawArrays` marker - without touching this path's behaviour.
|
||||
// `static` on purpose (G1): the pull build must not gain an exported name for a
|
||||
// function that only re-homes two identical inline blocks the two DrawArrays entry
|
||||
// points used to carry.
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
// ---- W3: THE MONOLITH ARM'S CLIENT-MEMORY SNAPSHOT, PER DRAW -------------------------
|
||||
// points used to carry. The same spelling governs the three below, and for a second
|
||||
// reason on top of the symbol count: a Debug build drops CXX_VISIBILITY_PRESET to
|
||||
// `default` (CMakeLists.txt:629-641), so an external-linkage helper here would be a
|
||||
// DYNAMIC export in exactly the configuration a symbol gate cannot see it in.
|
||||
//
|
||||
// ALL FLAVORS, NOT JUST THE SPLIT ARM. These three were inside
|
||||
// `#if MOBILEGL_BUILD_DISAGGREGATED` for one round and that was a misreading of their
|
||||
// own gate: the body's first statement returns early when `MG_Config::Transport !=
|
||||
// Monolith`, i.e. they exist FOR the monolith arm - and the monolith arm is what the
|
||||
// pull/verify/push builds run. Their callers sit in entry points that compile in every
|
||||
// flavor (the indexed family, both indirect executors), so a guard here breaks the
|
||||
// pull build outright rather than merely omitting a split-arm path. In those flavors
|
||||
// the guard's own test folds to a compile-time false (Config.h:566) and the body is
|
||||
// the transport-free upload it always was.
|
||||
//
|
||||
// ---- THE MONOLITH ARM'S CLIENT-MEMORY SNAPSHOT, PER DRAW -----------------------------
|
||||
//
|
||||
// A client-memory attribute is uploaded HERE because its bytes have no store on this side
|
||||
// to bind. SyncClientSideVertexArraysForDrawArrays below does that for the non-indexed
|
||||
@@ -2440,8 +2453,8 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
const SharedPtr<MG_State::GLState::BufferObject>* ElementBuffer = nullptr;
|
||||
};
|
||||
|
||||
Bool ReadClientSnapshotBytes(void* user, MG_Pipe::MGPipeClientBufferKind kind, Uint64 offset, SizeT size,
|
||||
void* destination) {
|
||||
static Bool ReadClientSnapshotBytes(void* user, MG_Pipe::MGPipeClientBufferKind kind, Uint64 offset, SizeT size,
|
||||
void* destination) {
|
||||
if (kind != MG_Pipe::MGPipeClientBufferKind::Element) return false;
|
||||
const auto& buffer = *static_cast<const ClientSnapshotSources*>(user)->ElementBuffer;
|
||||
if (!buffer || offset > buffer->GetSize() || size > buffer->GetSize() - offset) return false;
|
||||
@@ -2458,9 +2471,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// there is one. FALSE means the caller must SKIP the draw: the elements it would fetch are
|
||||
// not in the store the driver is about to read, and issuing it anyway is a wrong picture
|
||||
// rather than an error.
|
||||
Bool SyncClientSideVertexArraysForFetch(Uint8 indexSize, Uint32 elementStart, GLsizei count,
|
||||
GLsizei instanceCount, GLint baseVertex, Uint32 baseInstance,
|
||||
const void* clientIndices, Uint64 clientIndexBytes) {
|
||||
static Bool SyncClientSideVertexArraysForFetch(Uint8 indexSize, Uint32 elementStart, GLsizei count,
|
||||
GLsizei instanceCount, GLint baseVertex, Uint32 baseInstance,
|
||||
const void* clientIndices, Uint64 clientIndexBytes) {
|
||||
if (count <= 0 || instanceCount <= 0) return true;
|
||||
// MONOLITH ONLY, and the gate is what keeps a live transport's apply thread out of
|
||||
// the frontend VAO: with a transport the CLIENT owns those bytes and snapshots them
|
||||
@@ -2525,9 +2538,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// command block a shader wrote this frame is exactly what these shapes exist for - so the
|
||||
// command buffer's shadow is reconciled before it is read, and `commandBytes` (the
|
||||
// caller's possibly stale copy) is only the fallback for a client-authored block.
|
||||
Bool SyncClientSideVertexArraysForIndirectFetch(const Uint8* commandBytes, SizeT commandOffset,
|
||||
GLsizei stride, GLsizei index, Uint8 indexSize,
|
||||
const SharedPtr<MG_State::GLState::BufferObject>& commandBuffer) {
|
||||
static Bool SyncClientSideVertexArraysForIndirectFetch(
|
||||
const Uint8* commandBytes, SizeT commandOffset, GLsizei stride, GLsizei index, Uint8 indexSize,
|
||||
const SharedPtr<MG_State::GLState::BufferObject>& commandBuffer) {
|
||||
if (MG_Config::Transport != MG_Config::TransportMode::Monolith) return true;
|
||||
const Uint8* source = commandBytes;
|
||||
if (commandBuffer) {
|
||||
@@ -2554,8 +2567,9 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// The indexed entry points' form. `indices` is a byte offset into the bound element buffer
|
||||
// when one is bound and the application's own array otherwise, which is the same
|
||||
// distinction every one of them already makes when it issues the draw.
|
||||
Bool SyncClientSideVertexArraysForIndexedFetch(GLenum type, GLsizei count, const void* indices,
|
||||
GLsizei instanceCount, GLint baseVertex, GLuint baseInstance) {
|
||||
static Bool SyncClientSideVertexArraysForIndexedFetch(GLenum type, GLsizei count, const void* indices,
|
||||
GLsizei instanceCount, GLint baseVertex,
|
||||
GLuint baseInstance) {
|
||||
if (count <= 0 || instanceCount <= 0) return true;
|
||||
// MONOLITH ONLY - see SyncClientSideVertexArraysForFetch; this wrapper's own read of
|
||||
// the bound VAO is the frontend read that must not happen on an apply thread.
|
||||
@@ -2586,7 +2600,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
baseInstance, clientIndices ? indices : nullptr,
|
||||
clientIndexBytes);
|
||||
}
|
||||
#endif
|
||||
|
||||
static void SyncClientSideVertexArraysForDrawArrays(GLint first, GLsizei count) {
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
if (BufferImpl::VertexInputReadsRecords()) {
|
||||
|
||||
@@ -5545,7 +5545,6 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
}
|
||||
#endif // MOBILEGL_PIPE_PUSH
|
||||
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
// THE CLIENT-ARRAY UPLOAD FOR A DRAW WHOSE FETCHED ELEMENTS (first, count) DOES NOT
|
||||
// DESCRIBE. SyncClientSideAttributesForDrawArrays below answers the non-indexed family
|
||||
// exactly and nothing else: an indexed draw reads the elements its INDICES name, an
|
||||
@@ -5652,7 +5651,7 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
}
|
||||
return true;
|
||||
}
|
||||
#endif
|
||||
|
||||
void BackendVertexArrayObject::SyncClientSideAttributesForDrawArrays(
|
||||
const SharedPtr<MG_State::GLState::VertexArrayObject>& stateVAOObject, GLint first, GLsizei count,
|
||||
Uint32 fetchBaseInstance) {
|
||||
|
||||
@@ -17,13 +17,15 @@
|
||||
#include <MG_State/GLState/Core.h>
|
||||
#include <MG_Util/Converters/MGToGL/TextureEnumConverter.h>
|
||||
#include "SlotTables.h"
|
||||
// The client-memory fetch plan: which elements of an application-owned array a draw reads.
|
||||
// The monolith arm answers it here (SyncClientSideAttributesForDraw) and the wire arm asks
|
||||
// the same question of the same header (MG_Impl/Pipe/OwnedDrawInputs.h). UNGUARDED, because
|
||||
// the monolith arm is not a split-arm fallback: it is the arm the pull, verify and push
|
||||
// builds run, so the declaration below compiles in every flavor (see ClientFetchPlan.h).
|
||||
#include <MG_Impl/Pipe/ClientFetchPlan.h>
|
||||
#if MOBILEGL_PIPE_PUSH
|
||||
// P3a: the vertex-input payload views the handle arm of the VAO twin consumes.
|
||||
#include <MG_Pipe/MGPipeTypes.h>
|
||||
// The client-memory fetch plan: which elements of an application-owned array a draw reads.
|
||||
// The monolith arm answers it here (SyncClientSideAttributesForDraw) and the wire arm asks
|
||||
// the same question of the same header (MG_Impl/Pipe/OwnedDrawInputs.h).
|
||||
#include <MG_Impl/Pipe/ClientFetchPlan.h>
|
||||
// P4a: the RECORDS the five re-keyed twins read instead of the frontend object. The readers
|
||||
// below hand back pointers to them, and MGPipeResourceRecord::PendingUpload is a nested type,
|
||||
// so a forward declaration would not do. Push-only, like everything else P4a adds to this
|
||||
@@ -1345,7 +1347,6 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
void SyncClientSideAttributesForDrawArrays(
|
||||
const SharedPtr<MG_State::GLState::VertexArrayObject>& stateVAOObject, GLint first, GLsizei count,
|
||||
Uint32 fetchBaseInstance = 0);
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED
|
||||
// THE SAME UPLOAD FOR A DRAW WHOSE FETCHED ELEMENTS THE (first, count) RANGE DOES NOT
|
||||
// DESCRIBE: an indexed draw reads the elements its indices name, an instanced one one
|
||||
// element per instance, and the indirect forms a range that lives in GPU memory.
|
||||
@@ -1356,10 +1357,15 @@ namespace MobileGL::MG_Backend::DirectGLES {
|
||||
// side cannot read, a fetch range that cannot be represented). The caller must then
|
||||
// SKIP the draw: issuing it would have the shader read whatever the ES context last
|
||||
// held for that attribute, which is a wrong picture rather than an error.
|
||||
//
|
||||
// UNGUARDED WITH ITS CALLERS: the three VertexArrayImpl wrappers above it in
|
||||
// DirectGLES.cpp early-return unless the transport is Monolith, so this overload is
|
||||
// reached by the pull, verify and push build too - and those builds compile the
|
||||
// indexed and indirect entry points that call it. The declaration follows its
|
||||
// definition (Managers.cpp).
|
||||
Bool SyncClientSideAttributesForDraw(
|
||||
const SharedPtr<MG_State::GLState::VertexArrayObject>& stateVAOObject,
|
||||
const MG_Pipe::MGPipeClientFetchPlan& plan, Uint32 fetchBaseInstance);
|
||||
#endif
|
||||
Uint GetBackendVertexArrayId() const { return m_backendVAOId; }
|
||||
void Bind() const;
|
||||
|
||||
|
||||
@@ -23,9 +23,16 @@
|
||||
// INDICES; reading the bytes behind them is the caller's, because only the caller knows
|
||||
// whether the elements are read from the client's own array or out of a store the server
|
||||
// owns.
|
||||
//
|
||||
// COMPILED IN EVERY FLAVOR, and the guard it used to carry (`MOBILEGL_BUILD_DISAGGREGATED &&
|
||||
// MOBILEGL_PIPE_PUSH`) was the same mistake the monolith arm's callers carried: the
|
||||
// MONOLITH arm is not a split-arm fallback, it is the arm the pull build runs. An indexed or
|
||||
// indirect draw over a client-memory array staged nothing there either, so the question has
|
||||
// to be answerable - and answered - in the transport-free library too. The plan itself is
|
||||
// pure arithmetic over the draw's own words; what changes per flavor is only who reads the
|
||||
// bytes (the caller's reader), never the answer.
|
||||
#pragma once
|
||||
|
||||
#if MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH
|
||||
#include <MG_Pipe/MGPipeTypes.h>
|
||||
#include <MG_State/GLState/BufferState/BufferObject.h>
|
||||
#include <algorithm>
|
||||
@@ -237,4 +244,3 @@ namespace MobileGL::MG_Pipe {
|
||||
Vector<Uint64> m_vertices;
|
||||
};
|
||||
} // namespace MobileGL::MG_Pipe
|
||||
#endif // MOBILEGL_BUILD_DISAGGREGATED && MOBILEGL_PIPE_PUSH
|
||||
|
||||
Reference in New Issue
Block a user