From 39201af79fd51f25122c908ae02567db64f72014 Mon Sep 17 00:00:00 2001 From: Alex Szpakowski Date: Sun, 14 Jan 2018 19:15:24 -0400 Subject: [PATCH] Improve the error message when an invalid layer or face index is given in love.graphics.setMode. --- src/modules/graphics/Graphics.cpp | 35 +++++++++-- src/modules/graphics/Graphics.h | 2 +- src/modules/graphics/Texture.cpp | 17 ++++++ src/modules/graphics/Texture.h | 2 + src/modules/graphics/opengl/Graphics.cpp | 72 ++++++++--------------- src/modules/graphics/opengl/Graphics.h | 2 - src/modules/image/CompressedImageData.cpp | 2 +- src/modules/image/CompressedImageData.h | 2 +- 8 files changed, 76 insertions(+), 58 deletions(-) diff --git a/src/modules/graphics/Graphics.cpp b/src/modules/graphics/Graphics.cpp index 126a3e28a..3befca862 100644 --- a/src/modules/graphics/Graphics.cpp +++ b/src/modules/graphics/Graphics.cpp @@ -578,7 +578,10 @@ void Graphics::setCanvas(const RenderTargets &rts) DisplayState &state = states.back(); int ncanvases = (int) rts.colors.size(); - if (ncanvases == 0 && rts.depthStencil.canvas == nullptr) + RenderTarget firsttarget = rts.getFirstTarget(); + love::graphics::Canvas *firstcanvas = firsttarget.canvas; + + if (firstcanvas == nullptr) return setCanvas(); const auto &prevRTs = state.renderTargets; @@ -609,9 +612,6 @@ void Graphics::setCanvas(const RenderTargets &rts) if (ncanvases > capabilities.limits[LIMIT_MULTI_CANVAS]) throw love::Exception("This system can't simultaneously render to %d canvases.", ncanvases); - RenderTarget firsttarget = rts.getFirstTarget(); - love::graphics::Canvas *firstcanvas = firsttarget.canvas; - bool multiformatsupported = capabilities.features[FEATURE_MULTI_CANVAS_FORMATS]; PixelFormat firstcolorformat = PIXELFORMAT_UNKNOWN; @@ -624,6 +624,9 @@ void Graphics::setCanvas(const RenderTargets &rts) if (firsttarget.mipmap < 0 || firsttarget.mipmap >= firstcanvas->getMipmapCount()) throw love::Exception("Invalid mipmap level %d.", firsttarget.mipmap + 1); + if (!firstcanvas->isValidSlice(firsttarget.slice)) + throw love::Exception("Invalid slice index: %d.", firsttarget.slice + 1); + bool hasSRGBcanvas = firstcolorformat == PIXELFORMAT_sRGBA8; int pixelw = firstcanvas->getPixelWidth(firsttarget.mipmap); int pixelh = firstcanvas->getPixelHeight(firsttarget.mipmap); @@ -633,10 +636,14 @@ void Graphics::setCanvas(const RenderTargets &rts) love::graphics::Canvas *c = rts.colors[i].canvas; PixelFormat format = c->getPixelFormat(); int mip = rts.colors[i].mipmap; + int slice = rts.colors[i].slice; if (mip < 0 || mip >= c->getMipmapCount()) throw love::Exception("Invalid mipmap level %d.", mip + 1); + if (!c->isValidSlice(slice)) + throw love::Exception("Invalid slice index: %d.", slice + 1); + if (c->getPixelWidth(mip) != pixelw || c->getPixelHeight(mip) != pixelh) throw love::Exception("All canvases must have the same pixel dimensions."); @@ -657,6 +664,7 @@ void Graphics::setCanvas(const RenderTargets &rts) { love::graphics::Canvas *c = rts.depthStencil.canvas; int mip = rts.depthStencil.mipmap; + int slice = rts.depthStencil.slice; if (!isPixelFormatDepthStencil(c->getPixelFormat())) throw love::Exception("Only depth/stencil format Canvases can be used with the 'depthstencil' field of the table passed into setCanvas."); @@ -669,6 +677,9 @@ void Graphics::setCanvas(const RenderTargets &rts) if (mip < 0 || mip >= c->getMipmapCount()) throw love::Exception("Invalid mipmap level %d.", mip + 1); + + if (!c->isValidSlice(slice)) + throw love::Exception("Invalid slice index: %d.", slice + 1); } int w = firstcanvas->getWidth(firsttarget.mipmap); @@ -681,7 +692,7 @@ void Graphics::setCanvas(const RenderTargets &rts) refs.colors.reserve(rts.colors.size()); for (auto c : rts.colors) - refs.colors.emplace_back(c.canvas, c.slice); + refs.colors.emplace_back(c.canvas, c.slice, c.mipmap); refs.depthStencil = RenderTargetStrongRef(rts.depthStencil.canvas, rts.depthStencil.slice); refs.temporaryRTFlags = rts.temporaryRTFlags; @@ -691,6 +702,20 @@ void Graphics::setCanvas(const RenderTargets &rts) canvasSwitchCount++; } +void Graphics::setCanvas() +{ + DisplayState &state = states.back(); + + if (state.renderTargets.colors.empty() && state.renderTargets.depthStencil.canvas == nullptr) + return; + + flushStreamDraws(); + setCanvasInternal(RenderTargets(), width, height, pixelWidth, pixelHeight, isGammaCorrect()); + + state.renderTargets = RenderTargetsStrongRef(); + canvasSwitchCount++; +} + Graphics::RenderTargets Graphics::getCanvas() const { const auto &curRTs = states.back().renderTargets; diff --git a/src/modules/graphics/Graphics.h b/src/modules/graphics/Graphics.h index 37da61b02..439caa5b0 100644 --- a/src/modules/graphics/Graphics.h +++ b/src/modules/graphics/Graphics.h @@ -570,7 +570,7 @@ public: void setCanvas(RenderTarget rt, uint32 temporaryRTFlags); void setCanvas(const RenderTargets &rts); void setCanvas(const RenderTargetsStrongRef &rts); - virtual void setCanvas() = 0; + void setCanvas(); RenderTargets getCanvas() const; bool isCanvasActive() const; diff --git a/src/modules/graphics/Texture.cpp b/src/modules/graphics/Texture.cpp index 49ab45259..704695510 100644 --- a/src/modules/graphics/Texture.cpp +++ b/src/modules/graphics/Texture.cpp @@ -102,6 +102,23 @@ bool Texture::isReadable() const return readable; } +bool Texture::isValidSlice(int slice) const +{ + if (slice < 0) + return false; + + if (texType == TEXTURE_CUBE) + return slice < 6; + else if (texType == TEXTURE_VOLUME) + return slice < depth; + else if (texType == TEXTURE_2D_ARRAY) + return slice < layers; + else if (slice > 0) + return false; + + return true; +} + void Texture::draw(Graphics *gfx, const Matrix4 &m) { draw(gfx, quad, m); diff --git a/src/modules/graphics/Texture.h b/src/modules/graphics/Texture.h index f642f58a4..e27c2b947 100644 --- a/src/modules/graphics/Texture.h +++ b/src/modules/graphics/Texture.h @@ -120,6 +120,8 @@ public: bool isReadable() const; + bool isValidSlice(int slice) const; + int getWidth(int mip = 0) const; int getHeight(int mip = 0) const; int getDepth(int mip = 0) const; diff --git a/src/modules/graphics/opengl/Graphics.cpp b/src/modules/graphics/opengl/Graphics.cpp index 3fee0517f..f528c20c4 100644 --- a/src/modules/graphics/opengl/Graphics.cpp +++ b/src/modules/graphics/opengl/Graphics.cpp @@ -475,26 +475,42 @@ void Graphics::setCanvasInternal(const RenderTargets &rts, int w, int h, int pix { const DisplayState &state = states.back(); - OpenGL::TempDebugGroup debuggroup("setCanvas(...)"); + OpenGL::TempDebugGroup debuggroup("setCanvas"); flushStreamDraws(); endPass(); - bindCachedFBO(rts); + bool iswindow = rts.getFirstTarget().canvas == nullptr; + vertex::Winding vertexwinding = state.winding; + + if (iswindow) + { + gl.bindFramebuffer(OpenGL::FRAMEBUFFER_ALL, gl.getDefaultFBO()); + + // The projection matrix is flipped compared to rendering to a canvas, due + // to OpenGL considering (0,0) bottom-left instead of top-left. + projectionMatrix = Matrix4::ortho(0.0, (float) w, (float) h, 0.0, -10.0f, 10.0f); + } + else + { + bindCachedFBO(rts); + + projectionMatrix = Matrix4::ortho(0.0, (float) w, 0.0, (float) h, -10.0f, 10.0f); + + // Flip front face winding when rendering to a canvas, since our + // projection matrix is flipped. + vertexwinding = vertexwinding == vertex::WINDING_CW ? vertex::WINDING_CCW : vertex::WINDING_CW; + } + + glFrontFace(vertexwinding == vertex::WINDING_CW ? GL_CW : GL_CCW); gl.setViewport({0, 0, pixelw, pixelh}); - // Flip front face winding when rendering to a canvas, since our projection - // matrix is flipped. - glFrontFace(state.winding == vertex::WINDING_CW ? GL_CCW : GL_CW); - // Re-apply the scissor if it was active, since the rectangle passed to // glScissor is affected by the viewport dimensions. if (state.scissor) setScissor(state.scissorRect); - projectionMatrix = Matrix4::ortho(0.0, (float) w, 0.0, (float) h, -10.0f, 10.0f); - // Make sure the correct sRGB setting is used when drawing to the canvases. if (GLAD_VERSION_1_0 || GLAD_EXT_sRGB_write_control) { @@ -503,46 +519,6 @@ void Graphics::setCanvasInternal(const RenderTargets &rts, int w, int h, int pix } } -void Graphics::setCanvas() -{ - DisplayState &state = states.back(); - - if (state.renderTargets.colors.empty() && state.renderTargets.depthStencil.canvas == nullptr) - return; - - OpenGL::TempDebugGroup debuggroup("setCanvas()"); - - flushStreamDraws(); - endPass(); - - // Re-apply the correct front face winding, since it may have been flipped - // if we were previously rendering to a canvas. - glFrontFace(state.winding == vertex::WINDING_CW ? GL_CW : GL_CCW); - - state.renderTargets = RenderTargetsStrongRef(); - - gl.bindFramebuffer(OpenGL::FRAMEBUFFER_ALL, gl.getDefaultFBO()); - - gl.setViewport({0, 0, pixelWidth, pixelHeight}); - - // Re-apply the scissor if it was active, since the rectangle passed to - // glScissor is affected by the viewport dimensions. - if (state.scissor) - setScissor(state.scissorRect); - - // The projection matrix is flipped compared to rendering to a canvas, due - // to OpenGL considering (0,0) bottom-left instead of top-left. - projectionMatrix = Matrix4::ortho(0.0, (float) width, (float) height, 0.0, -10.0f, 10.0f); - - if (GLAD_VERSION_1_0 || GLAD_EXT_sRGB_write_control) - { - if (isGammaCorrect() != gl.isStateEnabled(OpenGL::ENABLE_FRAMEBUFFER_SRGB)) - gl.setEnableState(OpenGL::ENABLE_FRAMEBUFFER_SRGB, isGammaCorrect()); - } - - canvasSwitchCount++; -} - void Graphics::endPass() { auto &rts = states.back().renderTargets; diff --git a/src/modules/graphics/opengl/Graphics.h b/src/modules/graphics/opengl/Graphics.h index 7ba211655..338888744 100644 --- a/src/modules/graphics/opengl/Graphics.h +++ b/src/modules/graphics/opengl/Graphics.h @@ -82,8 +82,6 @@ public: void setColor(Colorf c) override; - void setCanvas() override; - void setScissor(const Rect &rect) override; void setScissor() override; diff --git a/src/modules/image/CompressedImageData.cpp b/src/modules/image/CompressedImageData.cpp index a3a452f4b..de32dff8c 100644 --- a/src/modules/image/CompressedImageData.cpp +++ b/src/modules/image/CompressedImageData.cpp @@ -91,7 +91,7 @@ void *CompressedImageData::getData() const return memory->data; } -int CompressedImageData::getMipmapCount(int /*slice*/) const +int CompressedImageData::getMipmapCount() const { return (int) dataImages.size(); } diff --git a/src/modules/image/CompressedImageData.h b/src/modules/image/CompressedImageData.h index 8b0c7895b..06919fd7d 100644 --- a/src/modules/image/CompressedImageData.h +++ b/src/modules/image/CompressedImageData.h @@ -61,7 +61,7 @@ public: * Gets the number of mipmaps in this Compressed Image Data. * Includes the base image level. **/ - int getMipmapCount(int slice = 0) const; + int getMipmapCount() const; /** * Gets the number of slices (array layers, cube faces, 3D layers, etc.)