mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-09-11 05:38:31 +09:00
[Fix] (MG_Impl): bound a colour attachment and a vertex binding range by the limit
GL_COLOR_ATTACHMENTn is a token for every n up to 31, but only the first GL_MAX_COLOR_ATTACHMENTS of them name an attachment point of a framebuffer object. The enum conversion accepted the whole token range, so attaching a renderbuffer or a texture to a colour attachment past the limit silently succeeded instead of reporting INVALID_OPERATION, and the attachment landed in a slot nothing else would ever look at. glBindVertexBuffers and glVertexArrayVertexBuffers take a range of binding points rather than one index. A range running past the last binding point is INVALID_OPERATION, which the per-binding validation could not report: it saw one index at a time and reported the INVALID_VALUE that a single out-of-range index earns. The range is checked up front now, before any binding point is touched, so a rejected call also leaves none of them changed. Takes direct_state_access.vertex_arrays_* to 18 of 19 and fixes direct_state_access.framebuffers_renderbuffer_attachment_errors on both backends.
This commit is contained in:
@@ -1143,6 +1143,7 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
|
|
||||||
const FramebufferAttachmentType attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment);
|
const FramebufferAttachmentType attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment);
|
||||||
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
||||||
|
if (!FramebufferImpl::ValidateColorAttachmentInRange(attachmentType, functionName)) return;
|
||||||
if (!TextureImpl::ValidateTextureName(texture, true)) return;
|
if (!TextureImpl::ValidateTextureName(texture, true)) return;
|
||||||
|
|
||||||
if (texture == 0) {
|
if (texture == 0) {
|
||||||
@@ -1239,6 +1240,7 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
FramebufferTarget framebufferTarget = MG_Util::ConvertGLEnumToFramebufferTarget(target);
|
FramebufferTarget framebufferTarget = MG_Util::ConvertGLEnumToFramebufferTarget(target);
|
||||||
RenderbufferTarget rbTarget = MG_Util::ConvertGLEnumToRenderbufferTarget(renderbuffertarget);
|
RenderbufferTarget rbTarget = MG_Util::ConvertGLEnumToRenderbufferTarget(renderbuffertarget);
|
||||||
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
||||||
|
if (!FramebufferImpl::ValidateColorAttachmentInRange(attachmentType, "FramebufferRenderbuffer_State")) return;
|
||||||
if (!FramebufferImpl::ValidateFramebufferTarget(framebufferTarget)) return;
|
if (!FramebufferImpl::ValidateFramebufferTarget(framebufferTarget)) return;
|
||||||
if (!FramebufferImpl::ValidateRenderbufferTarget(rbTarget)) return;
|
if (!FramebufferImpl::ValidateRenderbufferTarget(rbTarget)) return;
|
||||||
auto& bindingSlot = MG_State::pGLContext->GetFramebufferBindingSlot(framebufferTarget);
|
auto& bindingSlot = MG_State::pGLContext->GetFramebufferBindingSlot(framebufferTarget);
|
||||||
@@ -1283,6 +1285,8 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
FramebufferAttachmentType attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment);
|
FramebufferAttachmentType attachmentType = MG_Util::ConvertGLEnumToFramebufferAttachmentType(attachment);
|
||||||
RenderbufferTarget rbTarget = MG_Util::ConvertGLEnumToRenderbufferTarget(renderbuffertarget);
|
RenderbufferTarget rbTarget = MG_Util::ConvertGLEnumToRenderbufferTarget(renderbuffertarget);
|
||||||
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
if (!FramebufferImpl::ValidateFramebufferAttachmentType(attachmentType)) return;
|
||||||
|
if (!FramebufferImpl::ValidateColorAttachmentInRange(attachmentType, "NamedFramebufferRenderbuffer_State"))
|
||||||
|
return;
|
||||||
if (!FramebufferImpl::ValidateRenderbufferTarget(rbTarget)) return;
|
if (!FramebufferImpl::ValidateRenderbufferTarget(rbTarget)) return;
|
||||||
|
|
||||||
if (renderbuffer == 0) {
|
if (renderbuffer == 0) {
|
||||||
|
|||||||
@@ -7,6 +7,7 @@
|
|||||||
// End of Source File Header
|
// End of Source File Header
|
||||||
|
|
||||||
#include "Validators.h"
|
#include "Validators.h"
|
||||||
|
#include <MG_Backend/BackendObjects.h>
|
||||||
#include <MG_State/GLState/Core.h>
|
#include <MG_State/GLState/Core.h>
|
||||||
#include <MG_State/GLState/ErrorState/Error.h>
|
#include <MG_State/GLState/ErrorState/Error.h>
|
||||||
#include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
|
#include <MG_Util/Converters/GLToStr/GLEnumConverter.h>
|
||||||
@@ -60,6 +61,26 @@ namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Bool ValidateColorAttachmentInRange(FramebufferAttachmentType attachment, const char* caller) {
|
||||||
|
const auto first = static_cast<SizeT>(FramebufferAttachmentType::Color0);
|
||||||
|
const auto index = static_cast<SizeT>(attachment);
|
||||||
|
if (index < first) return true;
|
||||||
|
const auto colorIndex = index - first;
|
||||||
|
const auto limit = static_cast<SizeT>(
|
||||||
|
MG_Backend::pActiveBackendObject ? MG_Backend::pActiveBackendObject->GetDynamicParameters()
|
||||||
|
.MaxColorAttachments
|
||||||
|
: static_cast<Int>(MG_State::GLState::FramebufferObject::MAX_DRAW_BUFFERS));
|
||||||
|
if (colorIndex >= limit) {
|
||||||
|
MG_State::pGLContext->RecordError(
|
||||||
|
ErrorCode::InvalidOperation,
|
||||||
|
MakeUnique<GenericErrorInfo>(
|
||||||
|
"MG_Impl/GLImpl/FramebufferImpl", caller,
|
||||||
|
std::format("Colour attachment {} is beyond GL_MAX_COLOR_ATTACHMENTS ({}).", colorIndex, limit)));
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
Bool ValidateRenderbufferTarget(RenderbufferTarget target) {
|
Bool ValidateRenderbufferTarget(RenderbufferTarget target) {
|
||||||
if (target == RenderbufferTarget::Unknown) {
|
if (target == RenderbufferTarget::Unknown) {
|
||||||
using namespace MG_Util;
|
using namespace MG_Util;
|
||||||
|
|||||||
@@ -14,6 +14,10 @@ namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl {
|
|||||||
Bool ValidateFramebufferTarget(FramebufferTarget target);
|
Bool ValidateFramebufferTarget(FramebufferTarget target);
|
||||||
Bool ValidateFramebufferName(Uint index, Bool allowZero = true);
|
Bool ValidateFramebufferName(Uint index, Bool allowZero = true);
|
||||||
Bool ValidateFramebufferAttachmentType(FramebufferAttachmentType attachment);
|
Bool ValidateFramebufferAttachmentType(FramebufferAttachmentType attachment);
|
||||||
|
// GL_COLOR_ATTACHMENTn is a token per n up to 31, but only the first GL_MAX_COLOR_ATTACHMENTS of
|
||||||
|
// them name an attachment point of a framebuffer object; the rest are INVALID_OPERATION for the
|
||||||
|
// attaching entry points (GL 4.6 core 9.2.7). Non-colour attachments pass through unchanged.
|
||||||
|
Bool ValidateColorAttachmentInRange(FramebufferAttachmentType attachment, const char* caller);
|
||||||
Bool ValidateRenderbufferTarget(RenderbufferTarget target);
|
Bool ValidateRenderbufferTarget(RenderbufferTarget target);
|
||||||
Bool ValidateRenderbufferName(Uint index, Bool allowZero = true);
|
Bool ValidateRenderbufferName(Uint index, Bool allowZero = true);
|
||||||
} // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl
|
} // namespace MobileGL::MG_Impl::GLImpl::FramebufferImpl
|
||||||
|
|||||||
@@ -121,6 +121,27 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
return static_cast<int>(size * MG_Util::GetGLTypeSize(type));
|
return static_cast<int>(size * MG_Util::GetGLTypeSize(type));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// glBindVertexBuffers / glVertexArrayVertexBuffers take a range of binding points, and a
|
||||||
|
// range that runs past the last one is INVALID_OPERATION rather than the INVALID_VALUE a
|
||||||
|
// single out-of-range index gets (GL 4.6 core 10.3.1).
|
||||||
|
static bool ValidateVertexBindingRange(GLuint first, GLsizei count, const char* funcName) {
|
||||||
|
if (count < 0) {
|
||||||
|
MG_State::pGLContext->RecordError(
|
||||||
|
ErrorCode::InvalidValue,
|
||||||
|
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", funcName, "count must be non-negative."));
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
if (static_cast<Uint64>(first) + static_cast<Uint64>(count) >
|
||||||
|
VertexArrayImpl::GetMaxVertexAttribBindings()) {
|
||||||
|
MG_State::pGLContext->RecordError(
|
||||||
|
ErrorCode::InvalidOperation,
|
||||||
|
MakeUnique<GenericErrorInfo>("MG_Impl/GLImpl", funcName,
|
||||||
|
"first + count exceeds GL_MAX_VERTEX_ATTRIB_BINDINGS."));
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
static bool ValidateVertexBindingIndex(GLuint bindingindex, const char* funcName) {
|
static bool ValidateVertexBindingIndex(GLuint bindingindex, const char* funcName) {
|
||||||
if (bindingindex >= VertexArrayImpl::GetMaxVertexAttribBindings()) {
|
if (bindingindex >= VertexArrayImpl::GetMaxVertexAttribBindings()) {
|
||||||
MG_State::pGLContext->RecordError(
|
MG_State::pGLContext->RecordError(
|
||||||
@@ -408,6 +429,7 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
const GLintptr* offsets, const GLsizei* strides) {
|
const GLintptr* offsets, const GLsizei* strides) {
|
||||||
auto vao = GetNamedVertexArrayObject_State(vaobj, "VertexArrayVertexBuffers_State");
|
auto vao = GetNamedVertexArrayObject_State(vaobj, "VertexArrayVertexBuffers_State");
|
||||||
if (!vao) return;
|
if (!vao) return;
|
||||||
|
if (!ValidateVertexBindingRange(first, count, "VertexArrayVertexBuffers_State")) return;
|
||||||
for (GLsizei i = 0; i < count; ++i) {
|
for (GLsizei i = 0; i < count; ++i) {
|
||||||
if (!buffers) {
|
if (!buffers) {
|
||||||
VertexBufferBinding_State(vao, first + i, 0, 0, 16, "VertexArrayVertexBuffers_State");
|
VertexBufferBinding_State(vao, first + i, 0, 0, 16, "VertexArrayVertexBuffers_State");
|
||||||
@@ -1209,6 +1231,7 @@ namespace MobileGL::MG_Impl::GLImpl {
|
|||||||
const GLsizei* strides) {
|
const GLsizei* strides) {
|
||||||
auto vao = GetBoundVertexArrayOrError("BindVertexBuffers");
|
auto vao = GetBoundVertexArrayOrError("BindVertexBuffers");
|
||||||
if (!vao) return;
|
if (!vao) return;
|
||||||
|
if (!ValidateVertexBindingRange(first, count, "BindVertexBuffers")) return;
|
||||||
for (GLsizei i = 0; i < count; ++i) {
|
for (GLsizei i = 0; i < count; ++i) {
|
||||||
if (!buffers) {
|
if (!buffers) {
|
||||||
VertexBufferBinding_State(vao, first + i, 0, 0, 16, "BindVertexBuffers");
|
VertexBufferBinding_State(vao, first + i, 0, 0, 16, "BindVertexBuffers");
|
||||||
|
|||||||
Reference in New Issue
Block a user