From 2e7f5f28e7e9b75e114c828b6d1359fbf8b9ed64 Mon Sep 17 00:00:00 2001 From: Kelsey Gilbert Date: Mon, 7 Nov 2022 07:24:43 +0000 Subject: [PATCH] Bug 1798703 - Enforce alignment for UniformData via union rather than overalignment. r=lsalzman Reverts the overalignment from bug 1794237. Differential Revision: https://phabricator.services.mozilla.com/D161360 --- dom/canvas/ClientWebGLContext.cpp | 13 ++++++++----- dom/canvas/HostWebGLContext.h | 2 +- dom/canvas/QueueParamTraits.h | 7 +++++++ dom/canvas/WebGLChild.cpp | 23 ++++++++++++++++++----- dom/canvas/WebGLChild.h | 5 ++++- dom/canvas/WebGLCommandQueue.h | 38 +++++++++++++++++++++----------------- dom/canvas/WebGLContext.h | 2 +- dom/canvas/WebGLContextGL.cpp | 7 ++++--- dom/canvas/WebGLTypes.h | 7 +++++++ 9 files changed, 71 insertions(+), 33 deletions(-) diff --git a/dom/canvas/ClientWebGLContext.cpp b/dom/canvas/ClientWebGLContext.cpp index a83b283dfc29..4cdcfd467d6f 100644 --- a/dom/canvas/ClientWebGLContext.cpp +++ b/dom/canvas/ClientWebGLContext.cpp @@ -356,8 +356,9 @@ void ClientWebGLContext::Run(Args&&... args) const { const auto id = IdByMethod(); - const auto size = webgl::SerializedSize(id, args...); - const auto maybeDest = child->AllocPendingCmdBytes(size); + const auto info = webgl::SerializationInfo(id, args...); + const auto maybeDest = child->AllocPendingCmdBytes(info.requiredByteCount, + info.alignmentOverhead); if (!maybeDest) { JsWarning("Failed to allocate internal command buffer."); OnContextLoss(webgl::ContextLossReason::None); @@ -4710,9 +4711,11 @@ void ClientWebGLContext::UniformData(const GLenum funcElemType, // - - const auto ptr = bytes.begin().get() + (elemOffset * sizeof(float)); - const auto range = Range{ptr, availCount * sizeof(float)}; - Run(locId, transpose, RawBuffer<>(range)); + const auto begin = + reinterpret_cast(bytes.begin().get()) + + elemOffset; + const auto range = Range{begin, availCount}; + Run(locId, transpose, RawBuffer{range}); } // - diff --git a/dom/canvas/HostWebGLContext.h b/dom/canvas/HostWebGLContext.h index e76b1bc40cc5..ff97d97c4801 100644 --- a/dom/canvas/HostWebGLContext.h +++ b/dom/canvas/HostWebGLContext.h @@ -594,7 +594,7 @@ class HostWebGLContext final : public SupportsWeakPtr { // ------------------------ Uniforms and attributes ------------------------ void UniformData(uint32_t loc, bool transpose, - const RawBuffer<>& data) const { + const RawBuffer& data) const { mContext->UniformData(loc, transpose, data.Data()); } diff --git a/dom/canvas/QueueParamTraits.h b/dom/canvas/QueueParamTraits.h index 8af71a3ad7f3..b9ab1dea262e 100644 --- a/dom/canvas/QueueParamTraits.h +++ b/dom/canvas/QueueParamTraits.h @@ -95,6 +95,13 @@ static_assert(!BytesAlwaysValidT::value); // - +template <> +struct BytesAlwaysValidT { + static constexpr bool value = true; +}; + +// - + /** * Used to give QueueParamTraits a way to write to the Producer without * actually altering it, in case the transaction fails. diff --git a/dom/canvas/WebGLChild.cpp b/dom/canvas/WebGLChild.cpp index 70e072866b77..3724fdb56dc7 100644 --- a/dom/canvas/WebGLChild.cpp +++ b/dom/canvas/WebGLChild.cpp @@ -23,7 +23,8 @@ void WebGLChild::ActorDestroy(ActorDestroyReason why) { // - -Maybe> WebGLChild::AllocPendingCmdBytes(const size_t size) { +Maybe> WebGLChild::AllocPendingCmdBytes( + const size_t size, const size_t fyiAlignmentOverhead) { if (!mPendingCmdsShmem) { size_t capacity = mDefaultCmdsShmemSize; if (capacity < size) { @@ -37,6 +38,7 @@ Maybe> WebGLChild::AllocPendingCmdBytes(const size_t size) { } mPendingCmdsShmem = std::move(shmem); mPendingCmdsPos = 0; + mPendingCmdsAlignmentOverhead = 0; if (kIsDebug) { const auto range = mPendingCmdsShmem.ByteRange(); @@ -50,14 +52,16 @@ Maybe> WebGLChild::AllocPendingCmdBytes(const size_t size) { auto itr = range.begin() + mPendingCmdsPos; const auto offset = AlignmentOffset(kUniversalAlignment, itr.get()); mPendingCmdsPos += offset; + mPendingCmdsAlignmentOverhead += offset; const auto required = mPendingCmdsPos + size; if (required > range.length()) { FlushPendingCmds(); - return AllocPendingCmdBytes(size); + return AllocPendingCmdBytes(size, fyiAlignmentOverhead); } itr = range.begin() + mPendingCmdsPos; const auto remaining = Range{itr, range.end()}; mPendingCmdsPos += size; + mPendingCmdsAlignmentOverhead += fyiAlignmentOverhead; return Some(Range{remaining.begin(), remaining.begin() + size}); } @@ -69,11 +73,20 @@ void WebGLChild::FlushPendingCmds() { mFlushedCmdInfo.flushes += 1; mFlushedCmdInfo.flushedCmdBytes += byteSize; + mFlushedCmdInfo.overhead += mPendingCmdsAlignmentOverhead; if (gl::GLContext::ShouldSpew()) { - printf_stderr("[WebGLChild] Flushed %zu bytes. (%zu over %zu flushes)\n", - byteSize, mFlushedCmdInfo.flushedCmdBytes, - mFlushedCmdInfo.flushes); + const auto overheadRatio = float(mPendingCmdsAlignmentOverhead) / + (byteSize - mPendingCmdsAlignmentOverhead); + const auto totalOverheadRatio = + float(mFlushedCmdInfo.overhead) / + (mFlushedCmdInfo.flushedCmdBytes - mFlushedCmdInfo.overhead); + printf_stderr( + "[WebGLChild] Flushed %zu (%zu=%.2f%% overhead) bytes." + " (%zu (%.2f%% overhead) over %zu flushes)\n", + byteSize, mPendingCmdsAlignmentOverhead, 100 * overheadRatio, + mFlushedCmdInfo.flushedCmdBytes, 100 * totalOverheadRatio, + mFlushedCmdInfo.flushes); } } diff --git a/dom/canvas/WebGLChild.h b/dom/canvas/WebGLChild.h index cbac58a2f0b4..d5995045e3a5 100644 --- a/dom/canvas/WebGLChild.h +++ b/dom/canvas/WebGLChild.h @@ -20,6 +20,7 @@ namespace dom { struct FlushedCmdInfo final { size_t flushes = 0; size_t flushedCmdBytes = 0; + size_t overhead = 0; }; class WebGLChild final : public PWebGLChild, public SupportsWeakPtr { @@ -27,6 +28,7 @@ class WebGLChild final : public PWebGLChild, public SupportsWeakPtr { const size_t mDefaultCmdsShmemSize; webgl::RaiiShmem mPendingCmdsShmem; size_t mPendingCmdsPos = 0; + size_t mPendingCmdsAlignmentOverhead = 0; FlushedCmdInfo mFlushedCmdInfo; public: @@ -34,7 +36,8 @@ class WebGLChild final : public PWebGLChild, public SupportsWeakPtr { explicit WebGLChild(ClientWebGLContext&); - Maybe> AllocPendingCmdBytes(size_t); + Maybe> AllocPendingCmdBytes(size_t, + size_t fyiAlignmentOverhead); void FlushPendingCmds(); void ActorDestroy(ActorDestroyReason why) override; diff --git a/dom/canvas/WebGLCommandQueue.h b/dom/canvas/WebGLCommandQueue.h index c4270bdbc451..bbbb1034c1d2 100644 --- a/dom/canvas/WebGLCommandQueue.h +++ b/dom/canvas/WebGLCommandQueue.h @@ -30,18 +30,18 @@ class RangeConsumerView final : public webgl::ConsumerView { } void AlignTo(const size_t alignment) { - const auto offset = AlignmentOffset(alignment, mSrcItr.get()); - if (MOZ_UNLIKELY(offset > Remaining())) { + const auto padToAlign = AlignmentOffset(alignment, mSrcItr.get()); + if (MOZ_UNLIKELY(padToAlign > Remaining())) { mSrcItr = mSrcEnd; return; } - mSrcItr += offset; + mSrcItr += padToAlign; } template Maybe> ReadRange(const size_t elemCount) { - // uint32_t/float data may masquerade as a Range. - AlignTo(std::max(alignof(T), kUniversalAlignment)); + constexpr auto alignment = alignof(T); + AlignTo(alignment); constexpr auto elemSize = sizeof(T); const auto byteSizeChecked = CheckedInt(elemCount) * elemSize; @@ -63,26 +63,30 @@ namespace details { class SizeOnlyProducerView final : public webgl::ProducerView { - size_t mRequiredSize = 0; + struct Info { + size_t requiredByteCount = 0; + size_t alignmentOverhead = 0; + }; + Info mInfo; public: SizeOnlyProducerView() : ProducerView(this) {} template bool WriteFromRange(const Range& src) { - // uint32_t/float data may masquerade as a Range. - constexpr auto alignment = std::max(alignof(T), kUniversalAlignment); + constexpr auto alignment = alignof(T); const size_t byteSize = ByteSize(src); // printf_stderr("SizeOnlyProducerView: @%zu +%zu\n", alignment, byteSize); - const auto offset = AlignmentOffset(alignment, mRequiredSize); - mRequiredSize += offset; + const auto padToAlign = AlignmentOffset(alignment, mInfo.requiredByteCount); + mInfo.alignmentOverhead += padToAlign; - mRequiredSize += byteSize; + mInfo.requiredByteCount += padToAlign; + mInfo.requiredByteCount += byteSize; return true; } - const auto& RequiredSize() const { return mRequiredSize; } + const auto& Info() const { return mInfo; } }; // - @@ -106,12 +110,12 @@ class RangeProducerView final : public webgl::ProducerView { template bool WriteFromRange(const Range& src) { // uint32_t/float data may masquerade as a Range. - constexpr auto alignment = std::max(alignof(T), kUniversalAlignment); + constexpr auto alignment = alignof(T); const size_t byteSize = ByteSize(src); // printf_stderr("RangeProducerView: @%zu +%zu\n", alignment, byteSize); - const auto offset = AlignmentOffset(alignment, mDestItr.get()); - mDestItr += offset; + const auto padToAlign = AlignmentOffset(alignment, mDestItr.get()); + mDestItr += padToAlign; MOZ_ASSERT(byteSize <= Remaining()); if (MOZ_LIKELY(byteSize)) { @@ -139,10 +143,10 @@ inline void Serialize(ProducerViewT& view, const Arg& arg, // - template -size_t SerializedSize(const Args&... args) { +auto SerializationInfo(const Args&... args) { webgl::details::SizeOnlyProducerView sizeView; webgl::details::Serialize(sizeView, args...); - return sizeView.RequiredSize(); + return sizeView.Info(); } template diff --git a/dom/canvas/WebGLContext.h b/dom/canvas/WebGLContext.h index bb04af8c20e3..77b9270941b1 100644 --- a/dom/canvas/WebGLContext.h +++ b/dom/canvas/WebGLContext.h @@ -670,7 +670,7 @@ class WebGLContext : public VRefCounted, public SupportsWeakPtr { ////////////////////////// void UniformData(uint32_t loc, bool transpose, - const Range& data) const; + const Range& data) const; //////////////////////////////////// diff --git a/dom/canvas/WebGLContextGL.cpp b/dom/canvas/WebGLContextGL.cpp index c88cb3844d16..10b1f576ad68 100644 --- a/dom/canvas/WebGLContextGL.cpp +++ b/dom/canvas/WebGLContextGL.cpp @@ -1285,8 +1285,9 @@ void WebGLContext::StencilOpSeparate(GLenum face, GLenum sfail, GLenum dpfail, //////////////////////////////////////////////////////////////////////////////// // Uniform setters. -void WebGLContext::UniformData(const uint32_t loc, const bool transpose, - const Range& data) const { +void WebGLContext::UniformData( + const uint32_t loc, const bool transpose, + const Range& data) const { const FuncScope funcScope(*this, "uniform setter"); if (!IsWebGL2() && transpose) { @@ -1312,7 +1313,7 @@ void WebGLContext::UniformData(const uint32_t loc, const bool transpose, // - - const auto lengthInType = data.length() / sizeof(float); + const auto lengthInType = data.length(); const auto elemCount = lengthInType / channels; if (elemCount > 1 && !validationInfo.isArray) { GenerateError( diff --git a/dom/canvas/WebGLTypes.h b/dom/canvas/WebGLTypes.h index 794bf3a5863b..ebfc534eff1d 100644 --- a/dom/canvas/WebGLTypes.h +++ b/dom/canvas/WebGLTypes.h @@ -1191,6 +1191,13 @@ namespace webgl { // now. // (http://opengl.gpuinfo.org/gl_stats_caps_single.php?listreportsbycap=GL_MAX_COLOR_ATTACHMENTS) inline constexpr size_t kMaxDrawBuffers = 8; + +union UniformDataVal { + float f32; + int32_t i32; + uint32_t u32; +}; + } // namespace webgl } // namespace mozilla -- 2.11.4.GIT