Commit 851fbedb by Cody Northrop Committed by Angle LUCI CQ

Vulkan: Switch viewport and scissor to dynamic state

Heavily based on http://crrev/c/1316888 Some apps are creating a large number of viewport combinations and are running out of graphics memory. This CL drops their graphics pipeline use from tens of thousands to tens. Performance testing shows little impact to application traces. Bug: b/190026813 Bug: angleproject:3143 Change-Id: Ib7415be1128f8fedae4a7ca72e067b2815201223 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2954925Reviewed-by: 's avatarShahbaz Youssefi <syoussefi@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
parent 4e1fd371
...@@ -456,6 +456,11 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk ...@@ -456,6 +456,11 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::ContextVk"); ANGLE_TRACE_EVENT0("gpu.angle", "ContextVk::ContextVk");
memset(&mClearColorValue, 0, sizeof(mClearColorValue)); memset(&mClearColorValue, 0, sizeof(mClearColorValue));
memset(&mClearDepthStencilValue, 0, sizeof(mClearDepthStencilValue)); memset(&mClearDepthStencilValue, 0, sizeof(mClearDepthStencilValue));
memset(&mViewport, 0, sizeof(mViewport));
memset(&mScissor, 0, sizeof(mScissor));
// Ensure viewport is within Vulkan requirements
vk::ClampViewport(&mViewport);
mNonIndexedDirtyBitsMask.set(); mNonIndexedDirtyBitsMask.set();
mNonIndexedDirtyBitsMask.reset(DIRTY_BIT_INDEX_BUFFER); mNonIndexedDirtyBitsMask.reset(DIRTY_BIT_INDEX_BUFFER);
...@@ -469,10 +474,12 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk ...@@ -469,10 +474,12 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
// Note that currently these dirty bits are set every time a new render pass command buffer is // Note that currently these dirty bits are set every time a new render pass command buffer is
// begun. However, using ANGLE's SecondaryCommandBuffer, the Vulkan command buffer (which is // begun. However, using ANGLE's SecondaryCommandBuffer, the Vulkan command buffer (which is
// the primary command buffer) is not ended, so technically we don't need to rebind these. // the primary command buffer) is not ended, so technically we don't need to rebind these.
mNewGraphicsCommandBufferDirtyBits = DirtyBits{ mNewGraphicsCommandBufferDirtyBits =
DIRTY_BIT_RENDER_PASS, DIRTY_BIT_PIPELINE_BINDING, DIRTY_BIT_TEXTURES, DirtyBits{DIRTY_BIT_RENDER_PASS, DIRTY_BIT_PIPELINE_BINDING,
DIRTY_BIT_VERTEX_BUFFERS, DIRTY_BIT_INDEX_BUFFER, DIRTY_BIT_SHADER_RESOURCES, DIRTY_BIT_TEXTURES, DIRTY_BIT_VERTEX_BUFFERS,
DIRTY_BIT_DESCRIPTOR_SETS, DIRTY_BIT_DRIVER_UNIFORMS_BINDING}; DIRTY_BIT_INDEX_BUFFER, DIRTY_BIT_SHADER_RESOURCES,
DIRTY_BIT_DESCRIPTOR_SETS, DIRTY_BIT_DRIVER_UNIFORMS_BINDING,
DIRTY_BIT_VIEWPORT, DIRTY_BIT_SCISSOR};
if (getFeatures().supportsTransformFeedbackExtension.enabled) if (getFeatures().supportsTransformFeedbackExtension.enabled)
{ {
mNewGraphicsCommandBufferDirtyBits.set(DIRTY_BIT_TRANSFORM_FEEDBACK_BUFFERS); mNewGraphicsCommandBufferDirtyBits.set(DIRTY_BIT_TRANSFORM_FEEDBACK_BUFFERS);
...@@ -520,6 +527,9 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk ...@@ -520,6 +527,9 @@ ContextVk::ContextVk(const gl::State &state, gl::ErrorSet *errorSet, RendererVk
mGraphicsDirtyBitHandlers[DIRTY_BIT_DESCRIPTOR_SETS] = mGraphicsDirtyBitHandlers[DIRTY_BIT_DESCRIPTOR_SETS] =
&ContextVk::handleDirtyGraphicsDescriptorSets; &ContextVk::handleDirtyGraphicsDescriptorSets;
mGraphicsDirtyBitHandlers[DIRTY_BIT_VIEWPORT] = &ContextVk::handleDirtyGraphicsViewport;
mGraphicsDirtyBitHandlers[DIRTY_BIT_SCISSOR] = &ContextVk::handleDirtyGraphicsScissor;
mComputeDirtyBitHandlers[DIRTY_BIT_MEMORY_BARRIER] = mComputeDirtyBitHandlers[DIRTY_BIT_MEMORY_BARRIER] =
&ContextVk::handleDirtyComputeMemoryBarrier; &ContextVk::handleDirtyComputeMemoryBarrier;
mComputeDirtyBitHandlers[DIRTY_BIT_EVENT_LOG] = &ContextVk::handleDirtyComputeEventLog; mComputeDirtyBitHandlers[DIRTY_BIT_EVENT_LOG] = &ContextVk::handleDirtyComputeEventLog;
...@@ -1981,6 +1991,19 @@ angle::Result ContextVk::handleDirtyGraphicsDescriptorSets(DirtyBits::Iterator * ...@@ -1981,6 +1991,19 @@ angle::Result ContextVk::handleDirtyGraphicsDescriptorSets(DirtyBits::Iterator *
return handleDirtyDescriptorSetsImpl(mRenderPassCommandBuffer); return handleDirtyDescriptorSetsImpl(mRenderPassCommandBuffer);
} }
angle::Result ContextVk::handleDirtyGraphicsViewport(DirtyBits::Iterator *dirtyBitsIterator,
DirtyBits dirtyBitMask)
{
mRenderPassCommandBuffer->setViewport(0, 1, &mViewport);
return angle::Result::Continue;
}
angle::Result ContextVk::handleDirtyGraphicsScissor(DirtyBits::Iterator *dirtyBitsIterator,
DirtyBits dirtyBitMask)
{
mRenderPassCommandBuffer->setScissor(0, 1, &mScissor);
return angle::Result::Continue;
}
angle::Result ContextVk::handleDirtyComputeDescriptorSets() angle::Result ContextVk::handleDirtyComputeDescriptorSets()
{ {
return handleDirtyDescriptorSetsImpl(&mOutsideRenderPassCommands->getCommandBuffer()); return handleDirtyDescriptorSetsImpl(&mOutsideRenderPassCommands->getCommandBuffer());
...@@ -3216,7 +3239,6 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk, ...@@ -3216,7 +3239,6 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk,
bool invertViewport = bool invertViewport =
isViewportFlipEnabledForDrawFBO() && getFeatures().supportsNegativeViewport.enabled; isViewportFlipEnabledForDrawFBO() && getFeatures().supportsNegativeViewport.enabled;
VkViewport vkViewport;
gl_vk::GetViewport( gl_vk::GetViewport(
rotatedRect, nearPlane, farPlane, invertViewport, rotatedRect, nearPlane, farPlane, invertViewport,
// If clip space origin is upper left, viewport origin's y value will be offset by the // If clip space origin is upper left, viewport origin's y value will be offset by the
...@@ -3224,16 +3246,26 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk, ...@@ -3224,16 +3246,26 @@ void ContextVk::updateViewport(FramebufferVk *framebufferVk,
mState.getClipSpaceOrigin() == gl::ClipSpaceOrigin::UpperLeft, mState.getClipSpaceOrigin() == gl::ClipSpaceOrigin::UpperLeft,
// If the surface is rotated 90/270 degrees, use the framebuffer's width instead of the // If the surface is rotated 90/270 degrees, use the framebuffer's width instead of the
// height for calculating the final viewport. // height for calculating the final viewport.
isRotatedAspectRatioForDrawFBO() ? fbDimensions.width : fbDimensions.height, &vkViewport); isRotatedAspectRatioForDrawFBO() ? fbDimensions.width : fbDimensions.height, &mViewport);
// Ensure viewport is within Vulkan requirements
vk::ClampViewport(&mViewport);
mGraphicsPipelineDesc->updateViewport(&mGraphicsPipelineTransition, vkViewport);
invalidateGraphicsDriverUniforms(); invalidateGraphicsDriverUniforms();
mGraphicsDirtyBits.set(DIRTY_BIT_VIEWPORT);
} }
void ContextVk::updateDepthRange(float nearPlane, float farPlane) void ContextVk::updateDepthRange(float nearPlane, float farPlane)
{ {
// GLES2.0 Section 2.12.1: Each of n and f are clamped to lie within [0, 1], as are all
// arguments of type clampf.
ASSERT(nearPlane >= 0.0f && nearPlane <= 1.0f);
ASSERT(farPlane >= 0.0f && farPlane <= 1.0f);
mViewport.minDepth = nearPlane;
mViewport.maxDepth = farPlane;
invalidateGraphicsDriverUniforms(); invalidateGraphicsDriverUniforms();
mGraphicsPipelineDesc->updateDepthRange(&mGraphicsPipelineTransition, nearPlane, farPlane); mGraphicsDirtyBits.set(DIRTY_BIT_VIEWPORT);
} }
void ContextVk::updateScissor(const gl::State &glState) void ContextVk::updateScissor(const gl::State &glState)
...@@ -3253,9 +3285,8 @@ void ContextVk::updateScissor(const gl::State &glState) ...@@ -3253,9 +3285,8 @@ void ContextVk::updateScissor(const gl::State &glState)
gl::Rectangle rotatedScissoredArea; gl::Rectangle rotatedScissoredArea;
RotateRectangle(getRotationDrawFramebuffer(), isViewportFlipEnabledForDrawFBO(), RotateRectangle(getRotationDrawFramebuffer(), isViewportFlipEnabledForDrawFBO(),
renderArea.width, renderArea.height, scissoredArea, &rotatedScissoredArea); renderArea.width, renderArea.height, scissoredArea, &rotatedScissoredArea);
mScissor = gl_vk::GetRect(rotatedScissoredArea);
mGraphicsPipelineDesc->updateScissor(&mGraphicsPipelineTransition, mGraphicsDirtyBits.set(DIRTY_BIT_SCISSOR);
gl_vk::GetRect(rotatedScissoredArea));
// If the scissor has grown beyond the previous scissoredRenderArea, grow the render pass render // If the scissor has grown beyond the previous scissoredRenderArea, grow the render pass render
// area. The only undesirable effect this may have is that if the render area does not cover a // area. The only undesirable effect this may have is that if the render area does not cover a
...@@ -4347,6 +4378,12 @@ void ContextVk::invalidateComputeDescriptorSet(DescriptorSetIndex usedDescriptor ...@@ -4347,6 +4378,12 @@ void ContextVk::invalidateComputeDescriptorSet(DescriptorSetIndex usedDescriptor
} }
} }
void ContextVk::invalidateViewportAndScissor()
{
mGraphicsDirtyBits.set(DIRTY_BIT_VIEWPORT);
mGraphicsDirtyBits.set(DIRTY_BIT_SCISSOR);
}
angle::Result ContextVk::dispatchCompute(const gl::Context *context, angle::Result ContextVk::dispatchCompute(const gl::Context *context,
GLuint numGroupsX, GLuint numGroupsX,
GLuint numGroupsY, GLuint numGroupsY,
......
...@@ -368,6 +368,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -368,6 +368,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
void invalidateComputePipelineBinding(); void invalidateComputePipelineBinding();
void invalidateGraphicsDescriptorSet(DescriptorSetIndex usedDescriptorSet); void invalidateGraphicsDescriptorSet(DescriptorSetIndex usedDescriptorSet);
void invalidateComputeDescriptorSet(DescriptorSetIndex usedDescriptorSet); void invalidateComputeDescriptorSet(DescriptorSetIndex usedDescriptorSet);
void invalidateViewportAndScissor();
void optimizeRenderPassForPresent(VkFramebuffer framebufferHandle); void optimizeRenderPassForPresent(VkFramebuffer framebufferHandle);
...@@ -376,6 +377,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -376,6 +377,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
const VkClearValue &getClearColorValue() const; const VkClearValue &getClearColorValue() const;
const VkClearValue &getClearDepthStencilValue() const; const VkClearValue &getClearDepthStencilValue() const;
gl::BlendStateExt::ColorMaskStorage::Type getClearColorMasks() const; gl::BlendStateExt::ColorMaskStorage::Type getClearColorMasks() const;
const VkRect2D &getScissor() const { return mScissor; }
angle::Result getIncompleteTexture(const gl::Context *context, angle::Result getIncompleteTexture(const gl::Context *context,
gl::TextureType type, gl::TextureType type,
gl::SamplerFormat format, gl::SamplerFormat format,
...@@ -662,6 +664,9 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -662,6 +664,9 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
DIRTY_BIT_TRANSFORM_FEEDBACK_RESUME, DIRTY_BIT_TRANSFORM_FEEDBACK_RESUME,
DIRTY_BIT_DESCRIPTOR_SETS, DIRTY_BIT_DESCRIPTOR_SETS,
DIRTY_BIT_FRAMEBUFFER_FETCH_BARRIER, DIRTY_BIT_FRAMEBUFFER_FETCH_BARRIER,
// Dynamic viewport/scissor
DIRTY_BIT_VIEWPORT,
DIRTY_BIT_SCISSOR,
DIRTY_BIT_MAX, DIRTY_BIT_MAX,
}; };
...@@ -843,6 +848,10 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -843,6 +848,10 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
DirtyBits dirtyBitMask); DirtyBits dirtyBitMask);
angle::Result handleDirtyGraphicsDescriptorSets(DirtyBits::Iterator *dirtyBitsIterator, angle::Result handleDirtyGraphicsDescriptorSets(DirtyBits::Iterator *dirtyBitsIterator,
DirtyBits dirtyBitMask); DirtyBits dirtyBitMask);
angle::Result handleDirtyGraphicsViewport(DirtyBits::Iterator *dirtyBitsIterator,
DirtyBits dirtyBitMask);
angle::Result handleDirtyGraphicsScissor(DirtyBits::Iterator *dirtyBitsIterator,
DirtyBits dirtyBitMask);
// Handlers for compute pipeline dirty bits. // Handlers for compute pipeline dirty bits.
angle::Result handleDirtyComputeMemoryBarrier(); angle::Result handleDirtyComputeMemoryBarrier();
...@@ -865,6 +874,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -865,6 +874,7 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
VkPipelineBindPoint bindPoint, VkPipelineBindPoint bindPoint,
DriverUniformsDescriptorSet *driverUniforms); DriverUniformsDescriptorSet *driverUniforms);
angle::Result handleDirtyDescriptorSetsImpl(vk::CommandBuffer *commandBuffer); angle::Result handleDirtyDescriptorSetsImpl(vk::CommandBuffer *commandBuffer);
angle::Result allocateDriverUniforms(size_t driverUniformsSize, angle::Result allocateDriverUniforms(size_t driverUniformsSize,
DriverUniformsDescriptorSet *driverUniforms, DriverUniformsDescriptorSet *driverUniforms,
uint8_t **ptrOut, uint8_t **ptrOut,
...@@ -1154,6 +1164,10 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText ...@@ -1154,6 +1164,10 @@ class ContextVk : public ContextImpl, public vk::Context, public MultisampleText
// Record GL API calls for debuggers // Record GL API calls for debuggers
std::vector<std::string> mEventLog; std::vector<std::string> mEventLog;
// Viewport and scissor are handled as dynamic state.
VkViewport mViewport;
VkRect2D mScissor;
}; };
ANGLE_INLINE angle::Result ContextVk::endRenderPassIfTransformFeedbackBuffer( ANGLE_INLINE angle::Result ContextVk::endRenderPassIfTransformFeedbackBuffer(
......
...@@ -118,6 +118,8 @@ const char *GetCommandString(CommandID id) ...@@ -118,6 +118,8 @@ const char *GetCommandString(CommandID id)
return "SetEvent"; return "SetEvent";
case CommandID::SetScissor: case CommandID::SetScissor:
return "SetScissor"; return "SetScissor";
case CommandID::SetViewport:
return "SetViewport";
case CommandID::WaitEvents: case CommandID::WaitEvents:
return "WaitEvents"; return "WaitEvents";
case CommandID::WriteTimestamp: case CommandID::WriteTimestamp:
...@@ -547,6 +549,13 @@ void SecondaryCommandBuffer::executeCommands(VkCommandBuffer cmdBuffer) ...@@ -547,6 +549,13 @@ void SecondaryCommandBuffer::executeCommands(VkCommandBuffer cmdBuffer)
vkCmdSetScissor(cmdBuffer, 0, 1, &params->scissor); vkCmdSetScissor(cmdBuffer, 0, 1, &params->scissor);
break; break;
} }
case CommandID::SetViewport:
{
const SetViewportParams *params =
getParamPtr<SetViewportParams>(currentCommand);
vkCmdSetViewport(cmdBuffer, 0, 1, &params->viewport);
break;
}
case CommandID::WaitEvents: case CommandID::WaitEvents:
{ {
const WaitEventsParams *params = getParamPtr<WaitEventsParams>(currentCommand); const WaitEventsParams *params = getParamPtr<WaitEventsParams>(currentCommand);
......
...@@ -77,6 +77,7 @@ enum class CommandID : uint16_t ...@@ -77,6 +77,7 @@ enum class CommandID : uint16_t
ResolveImage, ResolveImage,
SetEvent, SetEvent,
SetScissor, SetScissor,
SetViewport,
WaitEvents, WaitEvents,
WriteTimestamp, WriteTimestamp,
}; };
...@@ -421,6 +422,12 @@ struct SetScissorParams ...@@ -421,6 +422,12 @@ struct SetScissorParams
}; };
VERIFY_4_BYTE_ALIGNMENT(SetScissorParams) VERIFY_4_BYTE_ALIGNMENT(SetScissorParams)
struct SetViewportParams
{
VkViewport viewport;
};
VERIFY_4_BYTE_ALIGNMENT(SetViewportParams)
struct WaitEventsParams struct WaitEventsParams
{ {
uint32_t eventCount; uint32_t eventCount;
...@@ -648,6 +655,8 @@ class SecondaryCommandBuffer final : angle::NonCopyable ...@@ -648,6 +655,8 @@ class SecondaryCommandBuffer final : angle::NonCopyable
void setScissor(uint32_t firstScissor, uint32_t scissorCount, const VkRect2D *scissors); void setScissor(uint32_t firstScissor, uint32_t scissorCount, const VkRect2D *scissors);
void setViewport(uint32_t firstViewport, uint32_t viewportCount, const VkViewport *viewports);
void waitEvents(uint32_t eventCount, void waitEvents(uint32_t eventCount,
const VkEvent *events, const VkEvent *events,
VkPipelineStageFlags srcStageMask, VkPipelineStageFlags srcStageMask,
...@@ -1394,6 +1403,17 @@ ANGLE_INLINE void SecondaryCommandBuffer::setScissor(uint32_t firstScissor, ...@@ -1394,6 +1403,17 @@ ANGLE_INLINE void SecondaryCommandBuffer::setScissor(uint32_t firstScissor,
paramStruct->scissor = scissors[0]; paramStruct->scissor = scissors[0];
} }
ANGLE_INLINE void SecondaryCommandBuffer::setViewport(uint32_t firstViewport,
uint32_t viewportCount,
const VkViewport *viewports)
{
ASSERT(firstViewport == 0);
ASSERT(viewportCount == 1);
ASSERT(viewports != nullptr);
SetViewportParams *paramStruct = initCommand<SetViewportParams>(CommandID::SetViewport);
paramStruct->viewport = viewports[0];
}
ANGLE_INLINE void SecondaryCommandBuffer::waitEvents( ANGLE_INLINE void SecondaryCommandBuffer::waitEvents(
uint32_t eventCount, uint32_t eventCount,
const VkEvent *events, const VkEvent *events,
......
...@@ -2076,12 +2076,12 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk, ...@@ -2076,12 +2076,12 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
const float clearDepthValue = params.depthStencilClearValue.depth; const float clearDepthValue = params.depthStencilClearValue.depth;
gl_vk::GetViewport(completeRenderArea, clearDepthValue, clearDepthValue, invertViewport, gl_vk::GetViewport(completeRenderArea, clearDepthValue, clearDepthValue, invertViewport,
clipSpaceOriginUpperLeft, completeRenderArea.height, &viewport); clipSpaceOriginUpperLeft, completeRenderArea.height, &viewport);
pipelineDesc.setViewport(viewport); commandBuffer->setViewport(0, 1, &viewport);
// Scissored clears can create a large number of pipelines in some tests. Use dynamic state for
// scissors.
pipelineDesc.setDynamicScissor();
const VkRect2D scissor = gl_vk::GetRect(params.clearArea); const VkRect2D scissor = gl_vk::GetRect(params.clearArea);
commandBuffer->setScissor(0, 1, &scissor);
contextVk->invalidateViewportAndScissor();
vk::ShaderLibrary &shaderLibrary = contextVk->getShaderLibrary(); vk::ShaderLibrary &shaderLibrary = contextVk->getShaderLibrary();
vk::RefCounted<vk::ShaderAndSerial> *vertexShader = nullptr; vk::RefCounted<vk::ShaderAndSerial> *vertexShader = nullptr;
...@@ -2108,7 +2108,7 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk, ...@@ -2108,7 +2108,7 @@ angle::Result UtilsVk::clearFramebuffer(ContextVk *contextVk,
// Make sure this draw call doesn't count towards occlusion query results. // Make sure this draw call doesn't count towards occlusion query results.
contextVk->pauseRenderPassQueriesIfActive(); contextVk->pauseRenderPassQueriesIfActive();
commandBuffer->setScissor(0, 1, &scissor);
commandBuffer->draw(3, 0); commandBuffer->draw(3, 0);
ANGLE_TRY(contextVk->resumeRenderPassQueriesIfActive()); ANGLE_TRY(contextVk->resumeRenderPassQueriesIfActive());
...@@ -2301,16 +2301,20 @@ angle::Result UtilsVk::blitResolveImpl(ContextVk *contextVk, ...@@ -2301,16 +2301,20 @@ angle::Result UtilsVk::blitResolveImpl(ContextVk *contextVk,
SetStencilForShaderExport(contextVk, &pipelineDesc); SetStencilForShaderExport(contextVk, &pipelineDesc);
} }
vk::CommandBuffer *commandBuffer;
ANGLE_TRY(framebuffer->startNewRenderPass(contextVk, params.blitArea, &commandBuffer, nullptr));
VkViewport viewport; VkViewport viewport;
gl::Rectangle completeRenderArea = framebuffer->getRotatedCompleteRenderArea(contextVk); gl::Rectangle completeRenderArea = framebuffer->getRotatedCompleteRenderArea(contextVk);
gl_vk::GetViewport(completeRenderArea, 0.0f, 1.0f, false, false, completeRenderArea.height, gl_vk::GetViewport(completeRenderArea, 0.0f, 1.0f, false, false, completeRenderArea.height,
&viewport); &viewport);
pipelineDesc.setViewport(viewport); commandBuffer->setViewport(0, 1, &viewport);
pipelineDesc.setScissor(gl_vk::GetRect(params.blitArea)); VkRect2D scissor = gl_vk::GetRect(params.blitArea);
commandBuffer->setScissor(0, 1, &scissor);
contextVk->invalidateViewportAndScissor();
vk::CommandBuffer *commandBuffer;
ANGLE_TRY(framebuffer->startNewRenderPass(contextVk, params.blitArea, &commandBuffer, nullptr));
contextVk->onImageRenderPassRead(src->getAspectFlags(), vk::ImageLayout::FragmentShaderReadOnly, contextVk->onImageRenderPassRead(src->getAspectFlags(), vk::ImageLayout::FragmentShaderReadOnly,
src); src);
...@@ -2737,16 +2741,18 @@ angle::Result UtilsVk::copyImage(ContextVk *contextVk, ...@@ -2737,16 +2741,18 @@ angle::Result UtilsVk::copyImage(ContextVk *contextVk,
std::swap(renderArea.width, renderArea.height); std::swap(renderArea.width, renderArea.height);
} }
vk::CommandBuffer *commandBuffer;
ANGLE_TRY(
startRenderPass(contextVk, dest, destView, renderPassDesc, renderArea, &commandBuffer));
VkViewport viewport; VkViewport viewport;
gl_vk::GetViewport(renderArea, 0.0f, 1.0f, false, false, dest->getExtents().height, &viewport); gl_vk::GetViewport(renderArea, 0.0f, 1.0f, false, false, dest->getExtents().height, &viewport);
pipelineDesc.setViewport(viewport); commandBuffer->setViewport(0, 1, &viewport);
VkRect2D scissor = gl_vk::GetRect(renderArea); VkRect2D scissor = gl_vk::GetRect(renderArea);
pipelineDesc.setScissor(scissor); commandBuffer->setScissor(0, 1, &scissor);
vk::CommandBuffer *commandBuffer; contextVk->invalidateViewportAndScissor();
ANGLE_TRY(
startRenderPass(contextVk, dest, destView, renderPassDesc, renderArea, &commandBuffer));
// Change source layout inside render pass. // Change source layout inside render pass.
contextVk->onImageRenderPassRead(VK_IMAGE_ASPECT_COLOR_BIT, contextVk->onImageRenderPassRead(VK_IMAGE_ASPECT_COLOR_BIT,
...@@ -3190,6 +3196,9 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk, ...@@ -3190,6 +3196,9 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk,
SetStencilForShaderExport(contextVk, &pipelineDesc); SetStencilForShaderExport(contextVk, &pipelineDesc);
} }
vk::CommandBuffer *commandBuffer =
&contextVk->getStartedRenderPassCommands().getCommandBuffer();
VkViewport viewport; VkViewport viewport;
gl::Rectangle completeRenderArea = framebuffer->getRotatedCompleteRenderArea(contextVk); gl::Rectangle completeRenderArea = framebuffer->getRotatedCompleteRenderArea(contextVk);
bool invertViewport = contextVk->isViewportFlipEnabledForDrawFBO(); bool invertViewport = contextVk->isViewportFlipEnabledForDrawFBO();
...@@ -3197,9 +3206,12 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk, ...@@ -3197,9 +3206,12 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk,
contextVk->getState().getClipSpaceOrigin() == gl::ClipSpaceOrigin::UpperLeft; contextVk->getState().getClipSpaceOrigin() == gl::ClipSpaceOrigin::UpperLeft;
gl_vk::GetViewport(completeRenderArea, 0.0f, 1.0f, invertViewport, clipSpaceOriginUpperLeft, gl_vk::GetViewport(completeRenderArea, 0.0f, 1.0f, invertViewport, clipSpaceOriginUpperLeft,
completeRenderArea.height, &viewport); completeRenderArea.height, &viewport);
pipelineDesc.setViewport(viewport); commandBuffer->setViewport(0, 1, &viewport);
VkRect2D scissor = gl_vk::GetRect(completeRenderArea);
commandBuffer->setScissor(0, 1, &scissor);
pipelineDesc.setScissor(gl_vk::GetRect(completeRenderArea)); contextVk->invalidateViewportAndScissor();
VkDescriptorSet descriptorSet; VkDescriptorSet descriptorSet;
vk::RefCountedDescriptorPoolBinding descriptorPoolBinding; vk::RefCountedDescriptorPoolBinding descriptorPoolBinding;
...@@ -3248,9 +3260,6 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk, ...@@ -3248,9 +3260,6 @@ angle::Result UtilsVk::unresolve(ContextVk *contextVk,
ANGLE_TRY(GetUnresolveFrag(contextVk, colorAttachmentCount, colorAttachmentTypes, ANGLE_TRY(GetUnresolveFrag(contextVk, colorAttachmentCount, colorAttachmentTypes,
params.unresolveDepth, params.unresolveStencil, fragmentShader)); params.unresolveDepth, params.unresolveStencil, fragmentShader));
vk::CommandBuffer *commandBuffer =
&contextVk->getStartedRenderPassCommands().getCommandBuffer();
ANGLE_TRY(setupProgram(contextVk, function, fragmentShader, vertexShader, ANGLE_TRY(setupProgram(contextVk, function, fragmentShader, vertexShader,
&mUnresolvePrograms[flags], &pipelineDesc, descriptorSet, nullptr, 0, &mUnresolvePrograms[flags], &pipelineDesc, descriptorSet, nullptr, 0,
commandBuffer)); commandBuffer));
......
...@@ -31,10 +31,6 @@ namespace vk ...@@ -31,10 +31,6 @@ namespace vk
namespace namespace
{ {
bool IsScissorStateDynamic(const PackedScissor &scissor)
{
return scissor.x == kDynamicScissorSentinel;
}
uint8_t PackGLBlendOp(GLenum blendOp) uint8_t PackGLBlendOp(GLenum blendOp)
{ {
...@@ -1696,19 +1692,6 @@ void GraphicsPipelineDesc::initDefaults(const ContextVk *contextVk) ...@@ -1696,19 +1692,6 @@ void GraphicsPipelineDesc::initDefaults(const ContextVk *contextVk)
SetBitField(inputAndBlend.primitive.patchVertices, 3); SetBitField(inputAndBlend.primitive.patchVertices, 3);
inputAndBlend.primitive.restartEnable = 0; inputAndBlend.primitive.restartEnable = 0;
// Viewport and scissor will be set to valid values when framebuffer being binded
mViewport.x = 0.0f;
mViewport.y = 0.0f;
mViewport.width = 0.0f;
mViewport.height = 0.0f;
mViewport.minDepth = 0.0f;
mViewport.maxDepth = 1.0f;
mScissor.x = 0;
mScissor.y = 0;
mScissor.width = 0;
mScissor.height = 0;
mDrawableSize.width = 1; mDrawableSize.width = 1;
mDrawableSize.height = 1; mDrawableSize.height = 1;
} }
...@@ -1921,38 +1904,12 @@ angle::Result GraphicsPipelineDesc::initializePipeline( ...@@ -1921,38 +1904,12 @@ angle::Result GraphicsPipelineDesc::initializePipeline(
static_cast<VkBool32>(mInputAssemblyAndColorBlendStateInfo.primitive.restartEnable); static_cast<VkBool32>(mInputAssemblyAndColorBlendStateInfo.primitive.restartEnable);
// Set initial viewport and scissor state. // Set initial viewport and scissor state.
// 0-sized viewports are invalid in Vulkan. We always use a scissor that at least matches the
// requested viewport, so it's safe to adjust the viewport size here.
VkViewport viewport = mViewport;
if (viewport.width == 0)
{
viewport.width = 1;
}
if (viewport.height == 0)
{
viewport.height = 1;
}
viewportState.sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO; viewportState.sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO;
viewportState.flags = 0; viewportState.flags = 0;
viewportState.viewportCount = 1; viewportState.viewportCount = 1;
viewportState.pViewports = &viewport; viewportState.pViewports = nullptr;
viewportState.scissorCount = 1;
viewportState.scissorCount = 1; viewportState.pScissors = nullptr;
VkRect2D scissor;
if (IsScissorStateDynamic(mScissor))
{
viewportState.pScissors = nullptr;
}
else
{
viewportState.pScissors = &scissor;
scissor.offset.x = mScissor.x;
scissor.offset.y = mScissor.y;
scissor.extent.width = mScissor.width;
scissor.extent.height = mScissor.height;
}
const PackedRasterizationAndMultisampleStateInfo &rasterAndMS = const PackedRasterizationAndMultisampleStateInfo &rasterAndMS =
mRasterizationAndMultisampleStateInfo; mRasterizationAndMultisampleStateInfo;
...@@ -2114,11 +2071,9 @@ angle::Result GraphicsPipelineDesc::initializePipeline( ...@@ -2114,11 +2071,9 @@ angle::Result GraphicsPipelineDesc::initializePipeline(
} }
// Dynamic state // Dynamic state
angle::FixedVector<VkDynamicState, 1> dynamicStateList; angle::FixedVector<VkDynamicState, 2> dynamicStateList;
if (IsScissorStateDynamic(mScissor)) dynamicStateList.push_back(VK_DYNAMIC_STATE_VIEWPORT);
{ dynamicStateList.push_back(VK_DYNAMIC_STATE_SCISSOR);
dynamicStateList.push_back(VK_DYNAMIC_STATE_SCISSOR);
}
VkPipelineDynamicStateCreateInfo dynamicState = {}; VkPipelineDynamicStateCreateInfo dynamicState = {};
dynamicState.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO; dynamicState.sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO;
...@@ -2620,67 +2575,6 @@ void GraphicsPipelineDesc::setRenderPassDesc(const RenderPassDesc &renderPassDes ...@@ -2620,67 +2575,6 @@ void GraphicsPipelineDesc::setRenderPassDesc(const RenderPassDesc &renderPassDes
mRenderPassDesc = renderPassDesc; mRenderPassDesc = renderPassDesc;
} }
void GraphicsPipelineDesc::setViewport(const VkViewport &viewport)
{
mViewport = viewport;
}
void GraphicsPipelineDesc::updateViewport(GraphicsPipelineTransitionBits *transition,
const VkViewport &viewport)
{
mViewport = viewport;
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, x));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, y));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, width));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, height));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, minDepth));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, maxDepth));
}
void GraphicsPipelineDesc::updateDepthRange(GraphicsPipelineTransitionBits *transition,
float nearPlane,
float farPlane)
{
// GLES2.0 Section 2.12.1: Each of n and f are clamped to lie within [0, 1], as are all
// arguments of type clampf.
ASSERT(nearPlane >= 0.0f && nearPlane <= 1.0f);
ASSERT(farPlane >= 0.0f && farPlane <= 1.0f);
mViewport.minDepth = nearPlane;
mViewport.maxDepth = farPlane;
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, minDepth));
transition->set(ANGLE_GET_TRANSITION_BIT(mViewport, maxDepth));
}
void GraphicsPipelineDesc::setDynamicScissor()
{
mScissor.x = kDynamicScissorSentinel;
mScissor.y = 0;
mScissor.width = 0;
mScissor.height = 0;
}
void GraphicsPipelineDesc::setScissor(const VkRect2D &scissor)
{
ASSERT(scissor.offset.x < kDynamicScissorSentinel &&
scissor.offset.y < kDynamicScissorSentinel &&
scissor.extent.width < kDynamicScissorSentinel &&
scissor.extent.height < kDynamicScissorSentinel);
SetBitField(mScissor.x, scissor.offset.x);
SetBitField(mScissor.y, scissor.offset.y);
SetBitField(mScissor.width, scissor.extent.width);
SetBitField(mScissor.height, scissor.extent.height);
}
void GraphicsPipelineDesc::updateScissor(GraphicsPipelineTransitionBits *transition,
const VkRect2D &scissor)
{
setScissor(scissor);
transition->set(ANGLE_GET_TRANSITION_BIT(mScissor, x));
transition->set(ANGLE_GET_TRANSITION_BIT(mScissor, y));
transition->set(ANGLE_GET_TRANSITION_BIT(mScissor, width));
transition->set(ANGLE_GET_TRANSITION_BIT(mScissor, height));
}
void GraphicsPipelineDesc::updateDrawableSize(GraphicsPipelineTransitionBits *transition, void GraphicsPipelineDesc::updateDrawableSize(GraphicsPipelineTransitionBits *transition,
uint32_t width, uint32_t width,
uint32_t height) uint32_t height)
......
...@@ -528,21 +528,11 @@ struct PackedInputAssemblyAndColorBlendStateInfo final ...@@ -528,21 +528,11 @@ struct PackedInputAssemblyAndColorBlendStateInfo final
PrimitiveState primitive; PrimitiveState primitive;
}; };
struct PackedScissor final
{
uint16_t x;
uint16_t y;
uint16_t width;
uint16_t height;
};
struct PackedExtent final struct PackedExtent final
{ {
uint16_t width; uint16_t width;
uint16_t height; uint16_t height;
}; };
// This is invalid value for PackedScissor.x. It is used to indicate scissor is a dynamic state
constexpr int32_t kDynamicScissorSentinel = std::numeric_limits<decltype(PackedScissor::x)>::max();
constexpr size_t kPackedInputAssemblyAndColorBlendStateSize = constexpr size_t kPackedInputAssemblyAndColorBlendStateSize =
sizeof(PackedInputAssemblyAndColorBlendStateInfo); sizeof(PackedInputAssemblyAndColorBlendStateInfo);
...@@ -550,8 +540,8 @@ static_assert(kPackedInputAssemblyAndColorBlendStateSize == 56, "Size check fail ...@@ -550,8 +540,8 @@ static_assert(kPackedInputAssemblyAndColorBlendStateSize == 56, "Size check fail
constexpr size_t kGraphicsPipelineDescSumOfSizes = constexpr size_t kGraphicsPipelineDescSumOfSizes =
kVertexInputAttributesSize + kRenderPassDescSize + kPackedRasterizationAndMultisampleStateSize + kVertexInputAttributesSize + kRenderPassDescSize + kPackedRasterizationAndMultisampleStateSize +
kPackedDepthStencilStateSize + kPackedInputAssemblyAndColorBlendStateSize + sizeof(VkViewport) + kPackedDepthStencilStateSize + kPackedInputAssemblyAndColorBlendStateSize +
sizeof(PackedScissor) + sizeof(PackedExtent); sizeof(PackedExtent);
// Number of dirty bits in the dirty bit set. // Number of dirty bits in the dirty bit set.
constexpr size_t kGraphicsPipelineDirtyBitBytes = 4; constexpr size_t kGraphicsPipelineDirtyBitBytes = 4;
...@@ -711,16 +701,6 @@ class GraphicsPipelineDesc final ...@@ -711,16 +701,6 @@ class GraphicsPipelineDesc final
void updatePolygonOffset(GraphicsPipelineTransitionBits *transition, void updatePolygonOffset(GraphicsPipelineTransitionBits *transition,
const gl::RasterizerState &rasterState); const gl::RasterizerState &rasterState);
// Viewport and scissor.
void setViewport(const VkViewport &viewport);
void updateViewport(GraphicsPipelineTransitionBits *transition, const VkViewport &viewport);
void updateDepthRange(GraphicsPipelineTransitionBits *transition,
float nearPlane,
float farPlane);
void setDynamicScissor();
void setScissor(const VkRect2D &scissor);
void updateScissor(GraphicsPipelineTransitionBits *transition, const VkRect2D &scissor);
// Tessellation // Tessellation
void updatePatchVertices(GraphicsPipelineTransitionBits *transition, GLuint value); void updatePatchVertices(GraphicsPipelineTransitionBits *transition, GLuint value);
...@@ -751,10 +731,6 @@ class GraphicsPipelineDesc final ...@@ -751,10 +731,6 @@ class GraphicsPipelineDesc final
PackedRasterizationAndMultisampleStateInfo mRasterizationAndMultisampleStateInfo; PackedRasterizationAndMultisampleStateInfo mRasterizationAndMultisampleStateInfo;
PackedDepthStencilStateInfo mDepthStencilStateInfo; PackedDepthStencilStateInfo mDepthStencilStateInfo;
PackedInputAssemblyAndColorBlendStateInfo mInputAssemblyAndColorBlendStateInfo; PackedInputAssemblyAndColorBlendStateInfo mInputAssemblyAndColorBlendStateInfo;
VkViewport mViewport;
// The special value of .offset.x == INT_MIN for scissor implies dynamic scissor that needs to
// be set through vkCmdSetScissor.
PackedScissor mScissor;
PackedExtent mDrawableSize; PackedExtent mDrawableSize;
}; };
......
...@@ -274,19 +274,6 @@ GLint LimitToInt(const LargerInt physicalDeviceValue) ...@@ -274,19 +274,6 @@ GLint LimitToInt(const LargerInt physicalDeviceValue)
physicalDeviceValue, static_cast<LargerInt>(std::numeric_limits<int32_t>::max() / 2))); physicalDeviceValue, static_cast<LargerInt>(std::numeric_limits<int32_t>::max() / 2)));
} }
template <typename LargerInt>
uint16_t LimitToDynamicScissorSentinelMinusOne(const LargerInt physicalDeviceValue)
{
static_assert(sizeof(LargerInt) >= sizeof(int32_t),
"Incorrect usage of LimitToDynamicScissorSentinelMinusOne");
// Limit to kDynamicScissorSentinel-1. This is used to pack drawable offset/dimension to
// uint16_t for space conservation. The UINT16_MAX is reserved for special value like
// kDynamicScissorSentinel.
return static_cast<uint16_t>(
std::min<int32_t>(physicalDeviceValue, vk::kDynamicScissorSentinel - 1));
}
void RendererVk::ensureCapsInitialized() const void RendererVk::ensureCapsInitialized() const
{ {
if (mCapsInitialized) if (mCapsInitialized)
...@@ -544,16 +531,12 @@ void RendererVk::ensureCapsInitialized() const ...@@ -544,16 +531,12 @@ void RendererVk::ensureCapsInitialized() const
mNativeCaps.maxDrawBuffers = mNativeCaps.maxDrawBuffers =
std::min(limitsVk.maxColorAttachments, limitsVk.maxFragmentOutputAttachments); std::min(limitsVk.maxColorAttachments, limitsVk.maxFragmentOutputAttachments);
mNativeCaps.maxFramebufferWidth = mNativeCaps.maxFramebufferWidth = LimitToInt(limitsVk.maxFramebufferWidth);
LimitToDynamicScissorSentinelMinusOne(limitsVk.maxFramebufferWidth); mNativeCaps.maxFramebufferHeight = LimitToInt(limitsVk.maxFramebufferHeight);
mNativeCaps.maxFramebufferHeight = mNativeCaps.maxColorAttachments = LimitToInt(limitsVk.maxColorAttachments);
LimitToDynamicScissorSentinelMinusOne(limitsVk.maxFramebufferHeight); mNativeCaps.maxViewportWidth = LimitToInt(limitsVk.maxViewportDimensions[0]);
mNativeCaps.maxColorAttachments = LimitToInt(limitsVk.maxColorAttachments); mNativeCaps.maxViewportHeight = LimitToInt(limitsVk.maxViewportDimensions[1]);
mNativeCaps.maxViewportWidth = mNativeCaps.maxSampleMaskWords = LimitToInt(limitsVk.maxSampleMaskWords);
LimitToDynamicScissorSentinelMinusOne(limitsVk.maxViewportDimensions[0]);
mNativeCaps.maxViewportHeight =
LimitToDynamicScissorSentinelMinusOne(limitsVk.maxViewportDimensions[1]);
mNativeCaps.maxSampleMaskWords = LimitToInt(limitsVk.maxSampleMaskWords);
mNativeCaps.maxColorTextureSamples = mNativeCaps.maxColorTextureSamples =
limitsVk.sampledImageColorSampleCounts & vk_gl::kSupportedSampleCounts; limitsVk.sampledImageColorSampleCounts & vk_gl::kSupportedSampleCounts;
mNativeCaps.maxDepthTextureSamples = mNativeCaps.maxDepthTextureSamples =
......
...@@ -813,6 +813,21 @@ uint32_t ResourceSerialFactory::issueSerial() ...@@ -813,6 +813,21 @@ uint32_t ResourceSerialFactory::issueSerial()
} }
ANGLE_VK_SERIAL_OP(ANGLE_DEFINE_GEN_VK_SERIAL) ANGLE_VK_SERIAL_OP(ANGLE_DEFINE_GEN_VK_SERIAL)
void ClampViewport(VkViewport *viewport)
{
// 0-sized viewports are invalid in Vulkan.
ASSERT(viewport);
if (viewport->width == 0.0f)
{
viewport->width = 1.0f;
}
if (viewport->height == 0.0f)
{
viewport->height = 1.0f;
}
}
} // namespace vk } // namespace vk
#if !defined(ANGLE_SHARED_LIBVULKAN) #if !defined(ANGLE_SHARED_LIBVULKAN)
......
...@@ -886,6 +886,10 @@ struct PerfCounters ...@@ -886,6 +886,10 @@ struct PerfCounters
// A Vulkan image level index. // A Vulkan image level index.
using LevelIndex = gl::LevelIndexWrapper<uint32_t>; using LevelIndex = gl::LevelIndexWrapper<uint32_t>;
// Ensure viewport is within Vulkan requirements
void ClampViewport(VkViewport *viewport);
} // namespace vk } // namespace vk
#if !defined(ANGLE_SHARED_LIBVULKAN) #if !defined(ANGLE_SHARED_LIBVULKAN)
......
...@@ -343,6 +343,7 @@ class CommandBuffer : public WrappedObject<CommandBuffer, VkCommandBuffer> ...@@ -343,6 +343,7 @@ class CommandBuffer : public WrappedObject<CommandBuffer, VkCommandBuffer>
const void *data); const void *data);
void setEvent(VkEvent event, VkPipelineStageFlags stageMask); void setEvent(VkEvent event, VkPipelineStageFlags stageMask);
void setViewport(uint32_t firstViewport, uint32_t viewportCount, const VkViewport *viewports);
void setScissor(uint32_t firstScissor, uint32_t scissorCount, const VkRect2D *scissors); void setScissor(uint32_t firstScissor, uint32_t scissorCount, const VkRect2D *scissors);
VkResult reset(); VkResult reset();
void resetEvent(VkEvent event, VkPipelineStageFlags stageMask); void resetEvent(VkEvent event, VkPipelineStageFlags stageMask);
...@@ -974,6 +975,14 @@ ANGLE_INLINE void CommandBuffer::setEvent(VkEvent event, VkPipelineStageFlags st ...@@ -974,6 +975,14 @@ ANGLE_INLINE void CommandBuffer::setEvent(VkEvent event, VkPipelineStageFlags st
vkCmdSetEvent(mHandle, event, stageMask); vkCmdSetEvent(mHandle, event, stageMask);
} }
ANGLE_INLINE void CommandBuffer::setViewport(uint32_t firstViewport,
uint32_t viewportCount,
const VkViewport *viewports)
{
ASSERT(valid() && viewports != nullptr);
vkCmdSetViewport(mHandle, firstViewport, viewportCount, viewports);
}
ANGLE_INLINE void CommandBuffer::setScissor(uint32_t firstScissor, ANGLE_INLINE void CommandBuffer::setScissor(uint32_t firstScissor,
uint32_t scissorCount, uint32_t scissorCount,
const VkRect2D *scissors) const VkRect2D *scissors)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment