Commit fd97c334 by Charlie Lao Committed by Commit Bot

Vulkan: Remove rotation related data from driver uniform

Rotation is now handled in the shader compiler with specialization constant, it should be removed from driver uniforms. Since Metal is using the flipXY, flipXY/negFlipXY are still kept in the shader side implementation, but have moved to TranslatorMetal in this CL. Bug: b/171750979 Change-Id: Ie8d15ef227cb52a6e19e4319ecc9f09bda42e667 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2519863Reviewed-by: 's avatarIan Elliott <ianelliott@google.com> Reviewed-by: 's avatarTim Van Patten <timvp@google.com> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Charlie Lao <cclao@google.com>
parent 4f96bf19
......@@ -341,9 +341,6 @@ const ShCompileOptions SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION = UINT64_C(1) << 55;
// Allow compiler to insert Android pre-rotation code.
const ShCompileOptions SH_ADD_PRE_ROTATION = UINT64_C(1) << 56;
// Allow compiler to use specialization constant to do pre-rotation and y flip.
const ShCompileOptions SH_USE_ROTATION_SPECIALIZATION_CONSTANT = UINT64_C(1) << 57;
// Defines alternate strategies for implementing array index clamping.
enum ShArrayIndexClampingStrategy
{
......
......@@ -42,10 +42,16 @@ const char kRasterizerDiscardEnabledConstName[] = "ANGLERasterizerDisabled";
namespace
{
// Metal specific driver uniforms
constexpr const char kFlipXY[] = "flipXY";
constexpr const char kNegFlipXY[] = "negFlipXY";
constexpr const char kCoverageMask[] = "coverageMask";
constexpr ImmutableString kSampleMaskWriteFuncName = ImmutableString("ANGLEWriteSampleMask");
constexpr size_t kNumMetalGraphicsDriverUniforms = 3;
constexpr std::array<const char *, kNumMetalGraphicsDriverUniforms>
kMetalGraphicsDriverUniformNames = {{kFlipXY, kNegFlipXY, kCoverageMask}};
// Unlike Vulkan having auto viewport flipping extension, in Metal we have to flip gl_Position.y
// manually.
// This operation performs flipping the gl_Position.y using this expression:
......@@ -111,25 +117,73 @@ ANGLE_NO_DISCARD bool InitializeUnusedOutputs(TIntermBlock *root,
}
} // anonymous namespace
// class DriverUniformMetal
TFieldList *DriverUniformMetal::createUniformFields(TSymbolTable *symbolTable) const
{
TFieldList *driverFieldList = DriverUniform::createUniformFields(symbolTable);
// Add coverage mask to driver uniform. Metal doesn't have built-in GL_SAMPLE_COVERAGE_VALUE
// equivalent functionality, needs to emulate it using fragment shader's [[sample_mask]] output
// value.
TField *coverageMaskField = new TField(new TType(EbtUInt), ImmutableString(kCoverageMask),
TSourceLoc(), SymbolType::AngleInternal);
driverFieldList->push_back(coverageMaskField);
const std::array<TType *, kNumMetalGraphicsDriverUniforms> kMetalDriverUniformTypes = {{
new TType(EbtFloat, 2), // flipXY
new TType(EbtFloat, 2), // negFlipXY
new TType(EbtUInt), // kCoverageMask
}};
for (size_t uniformIndex = 0; uniformIndex < kNumMetalGraphicsDriverUniforms; ++uniformIndex)
{
TField *driverUniformField =
new TField(kMetalDriverUniformTypes[uniformIndex],
ImmutableString(kMetalGraphicsDriverUniformNames[uniformIndex]),
TSourceLoc(), SymbolType::AngleInternal);
driverFieldList->push_back(driverUniformField);
}
return driverFieldList;
}
TIntermBinary *DriverUniformMetal::getFlipXYRef() const
{
return createDriverUniformRef(kFlipXY);
}
TIntermBinary *DriverUniformMetal::getNegFlipXYRef() const
{
return createDriverUniformRef(kNegFlipXY);
}
TIntermBinary *DriverUniformMetal::getCoverageMaskFieldRef() const
{
return createDriverUniformRef(kCoverageMask);
}
TIntermSwizzle *DriverUniformMetal::getNegFlipYRef() const
{
// Create a swizzle to "negFlipXY.y"
TIntermBinary *negFlipXY = createDriverUniformRef(kNegFlipXY);
TVector<int> swizzleOffsetY = {1};
TIntermSwizzle *negFlipY = new TIntermSwizzle(negFlipXY, swizzleOffsetY);
return negFlipY;
}
// class FlipRotateSpecConstMetal
TIntermTyped *FlipRotateSpecConstMetal::getFlipXY()
{
return mDriverUniform->getFlipXYRef();
}
TIntermTyped *FlipRotateSpecConstMetal::getNegFlipXY()
{
return mDriverUniform->getNegFlipXYRef();
}
TIntermTyped *FlipRotateSpecConstMetal::getFlipY()
{
TIntermTyped *flipXY = mDriverUniform->getFlipXYRef();
return new TIntermBinary(EOpIndexDirect, flipXY, CreateIndexNode(1));
}
TIntermTyped *FlipRotateSpecConstMetal::getNegFlipY()
{
return mDriverUniform->getNegFlipYRef();
}
// class TranslaterMetal
TranslatorMetal::TranslatorMetal(sh::GLenum type, ShShaderSpec spec) : TranslatorVulkan(type, spec)
{}
......@@ -144,8 +198,9 @@ bool TranslatorMetal::translate(TIntermBlock *root,
getShaderVersion(), getOutputType(), false, true, compileOptions);
DriverUniformMetal driverUniforms;
if (!TranslatorVulkan::translateImpl(root, compileOptions, perfDiagnostics, &driverUniforms,
&outputGLSL))
FlipRotateSpecConstMetal flipRotateSpecConst(&driverUniforms);
if (!TranslatorVulkan::translateImpl(root, compileOptions, perfDiagnostics,
&flipRotateSpecConst, &driverUniforms, &outputGLSL))
{
return false;
}
......@@ -338,5 +393,4 @@ ANGLE_NO_DISCARD bool TranslatorMetal::insertRasterizerDiscardLogic(TIntermBlock
return RunAtTheEndOfShader(this, root, ifCall, symbolTable);
}
} // namespace sh
......@@ -17,6 +17,7 @@
#include "compiler/translator/TranslatorVulkan.h"
#include "compiler/translator/tree_util/DriverUniform.h"
#include "compiler/translator/tree_util/FlipRotateSpecConst.h"
namespace sh
{
......@@ -27,12 +28,37 @@ class DriverUniformMetal : public DriverUniform
DriverUniformMetal() : DriverUniform() {}
~DriverUniformMetal() override {}
TIntermBinary *getFlipXYRef() const;
TIntermBinary *getNegFlipXYRef() const;
TIntermSwizzle *getNegFlipYRef() const;
TIntermBinary *getCoverageMaskFieldRef() const;
protected:
TFieldList *createUniformFields(TSymbolTable *symbolTable) const override;
};
// TODO: http://anglebug.com/5339 Implement it using actual specialization constant. For now we are
// redirecting to driver uniforms
class FlipRotateSpecConstMetal : public FlipRotateSpecConst
{
public:
FlipRotateSpecConstMetal(DriverUniformMetal *driverUniform)
: FlipRotateSpecConst(), mDriverUniform(driverUniform)
{}
~FlipRotateSpecConstMetal() override {}
TIntermTyped *getFlipXY() override;
TIntermTyped *getNegFlipXY() override;
TIntermTyped *getFlipY() override;
TIntermTyped *getNegFlipY() override;
void generateSymbol(TSymbolTable *symbolTable) override {}
void outputLayoutString(TInfoSinkBase &sink) const override {}
private:
DriverUniformMetal *mDriverUniform;
};
class TranslatorMetal : public TranslatorVulkan
{
public:
......
......@@ -317,14 +317,9 @@ ANGLE_NO_DISCARD bool AppendVertexShaderDepthCorrectionToMain(TCompiler *compile
ANGLE_NO_DISCARD bool AppendPreRotation(TCompiler *compiler,
TIntermBlock *root,
TSymbolTable *symbolTable,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
{
TIntermTyped *preRotationRef = rotationSpecConst->getPreRotationMatrix();
if (!preRotationRef)
{
preRotationRef = driverUniforms->getPreRotationMatrixRef();
}
TIntermSymbol *glPos = new TIntermSymbol(BuiltInVariable::gl_Position());
TVector<int> swizzleOffsetXY = {0, 1};
TIntermSwizzle *glPosXY = new TIntermSwizzle(glPos, swizzleOffsetXY);
......@@ -461,21 +456,11 @@ ANGLE_NO_DISCARD bool InsertFragCoordCorrection(TCompiler *compiler,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
{
TIntermTyped *flipXY = rotationSpecConst->getFlipXY();
if (!flipXY)
{
flipXY = driverUniforms->getFlipXYRef();
}
TIntermTyped *flipXY = rotationSpecConst->getFlipXY();
TIntermBinary *pivot = driverUniforms->getHalfRenderAreaRef();
TIntermTyped *fragRotation = nullptr;
if (compileOptions & SH_ADD_PRE_ROTATION)
{
fragRotation = rotationSpecConst->getFragRotationMatrix();
if (!fragRotation)
{
fragRotation = driverUniforms->getFragRotationMatrixRef();
}
}
TIntermTyped *fragRotation = (compileOptions & SH_ADD_PRE_ROTATION)
? rotationSpecConst->getFragRotationMatrix()
: nullptr;
return RotateAndFlipBuiltinVariable(compiler, root, insertSequence, flipXY, symbolTable,
BuiltInVariable::gl_FragCoord(), kFlippedFragCoordName,
pivot, fragRotation);
......@@ -635,6 +620,7 @@ TranslatorVulkan::TranslatorVulkan(sh::GLenum type, ShShaderSpec spec)
bool TranslatorVulkan::translateImpl(TIntermBlock *root,
ShCompileOptions compileOptions,
PerformanceDiagnostics * /*perfDiagnostics*/,
FlipRotateSpecConst *flipRotateSpecConst,
DriverUniform *driverUniforms,
TOutputVulkanGLSL *outputGLSL)
{
......@@ -754,11 +740,9 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
sink << "};\n";
}
FlipRotateSpecConst surfaceRotationSpecConst;
if (getShaderType() != GL_COMPUTE_SHADER &&
(compileOptions & SH_USE_ROTATION_SPECIALIZATION_CONSTANT))
if (getShaderType() != GL_COMPUTE_SHADER)
{
surfaceRotationSpecConst.generateSymbol(&getSymbolTable());
flipRotateSpecConst->generateSymbol(&getSymbolTable());
}
if (getShaderType() == GL_COMPUTE_SHADER)
......@@ -838,7 +822,7 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
if (compileOptions & SH_ADD_BRESENHAM_LINE_RASTER_EMULATION)
{
if (!AddBresenhamEmulationFS(this, compileOptions, sink, root, &getSymbolTable(),
&surfaceRotationSpecConst, driverUniforms, usesFragCoord))
flipRotateSpecConst, driverUniforms, usesFragCoord))
{
return false;
}
......@@ -875,21 +859,10 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
if (usesPointCoord)
{
TIntermTyped *flipNegXY = surfaceRotationSpecConst.getNegFlipXY();
if (!flipNegXY)
{
flipNegXY = driverUniforms->getNegFlipXYRef();
}
TIntermTyped *flipNegXY = flipRotateSpecConst->getNegFlipXY();
TIntermConstantUnion *pivot = CreateFloatNode(0.5f);
TIntermTyped *fragRotation = nullptr;
if (usePreRotation)
{
fragRotation = surfaceRotationSpecConst.getFragRotationMatrix();
if (!fragRotation)
{
fragRotation = driverUniforms->getFragRotationMatrixRef();
}
}
TIntermTyped *fragRotation =
usePreRotation ? flipRotateSpecConst->getFragRotationMatrix() : nullptr;
if (!RotateAndFlipBuiltinVariable(this, root, GetMainSequence(root), flipNegXY,
&getSymbolTable(), BuiltInVariable::gl_PointCoord(),
kFlippedPointCoordName, pivot, fragRotation))
......@@ -901,22 +874,20 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
if (usesFragCoord)
{
if (!InsertFragCoordCorrection(this, compileOptions, root, GetMainSequence(root),
&getSymbolTable(), &surfaceRotationSpecConst,
driverUniforms))
&getSymbolTable(), flipRotateSpecConst, driverUniforms))
{
return false;
}
}
if (!RewriteDfdy(this, compileOptions, root, getSymbolTable(), getShaderVersion(),
&surfaceRotationSpecConst, driverUniforms))
flipRotateSpecConst))
{
return false;
}
if (!RewriteInterpolateAtOffset(this, compileOptions, root, getSymbolTable(),
getShaderVersion(), &surfaceRotationSpecConst,
driverUniforms))
getShaderVersion(), flipRotateSpecConst))
{
return false;
}
......@@ -969,8 +940,7 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
return false;
}
if ((compileOptions & SH_ADD_PRE_ROTATION) != 0 &&
!AppendPreRotation(this, root, &getSymbolTable(), &surfaceRotationSpecConst,
driverUniforms))
!AppendPreRotation(this, root, &getSymbolTable(), flipRotateSpecConst))
{
return false;
}
......@@ -987,7 +957,7 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
EmitWorkGroupSizeGLSL(*this, sink);
}
surfaceRotationSpecConst.outputLayoutString(sink);
flipRotateSpecConst->outputLayoutString(sink);
if (!validateAST(root))
{
......@@ -1016,7 +986,9 @@ bool TranslatorVulkan::translate(TIntermBlock *root,
enablePrecision, compileOptions);
DriverUniform driverUniforms;
if (!translateImpl(root, compileOptions, perfDiagnostics, &driverUniforms, &outputGLSL))
FlipRotateSpecConst flipRotateSpecConst;
if (!translateImpl(root, compileOptions, perfDiagnostics, &flipRotateSpecConst, &driverUniforms,
&outputGLSL))
{
return false;
}
......
......@@ -19,6 +19,7 @@ namespace sh
class TOutputVulkanGLSL;
class DriverUniform;
class FlipRotateSpecConst;
class TranslatorVulkan : public TCompiler
{
......@@ -36,6 +37,7 @@ class TranslatorVulkan : public TCompiler
ANGLE_NO_DISCARD bool translateImpl(TIntermBlock *root,
ShCompileOptions compileOptions,
PerformanceDiagnostics *perfDiagnostics,
FlipRotateSpecConst *flipRotateSpecConst,
DriverUniform *driverUniforms,
TOutputVulkanGLSL *outputGLSL);
......
......@@ -11,7 +11,6 @@
#include "common/angleutils.h"
#include "compiler/translator/StaticType.h"
#include "compiler/translator/SymbolTable.h"
#include "compiler/translator/TranslatorVulkan.h"
#include "compiler/translator/tree_util/DriverUniform.h"
#include "compiler/translator/tree_util/FlipRotateSpecConst.h"
#include "compiler/translator/tree_util/IntermNode_util.h"
......@@ -30,31 +29,26 @@ class Traverser : public TIntermTraverser
ShCompileOptions compileOptions,
TIntermNode *root,
const TSymbolTable &symbolTable,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
private:
Traverser(TSymbolTable *symbolTable,
ShCompileOptions compileOptions,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
bool visitUnary(Visit visit, TIntermUnary *node) override;
bool visitUnaryWithRotation(Visit visit, TIntermUnary *node);
bool visitUnaryWithoutRotation(Visit visit, TIntermUnary *node);
FlipRotateSpecConst *mRotationSpecConst = nullptr;
const DriverUniform *mDriverUniforms = nullptr;
bool mUsePreRotation = false;
};
Traverser::Traverser(TSymbolTable *symbolTable,
ShCompileOptions compileOptions,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
: TIntermTraverser(true, false, false, symbolTable),
mRotationSpecConst(rotationSpecConst),
mDriverUniforms(driverUniforms),
mUsePreRotation((compileOptions & SH_ADD_PRE_ROTATION) != 0)
{}
......@@ -63,11 +57,10 @@ bool Traverser::Apply(TCompiler *compiler,
ShCompileOptions compileOptions,
TIntermNode *root,
const TSymbolTable &symbolTable,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
{
TSymbolTable *pSymbolTable = const_cast<TSymbolTable *>(&symbolTable);
Traverser traverser(pSymbolTable, compileOptions, rotationSpecConst, driverUniforms);
Traverser traverser(pSymbolTable, compileOptions, rotationSpecConst);
root->traverse(&traverser);
return traverser.updateTree(compiler, root);
}
......@@ -129,38 +122,6 @@ bool Traverser::visitUnaryWithRotation(Visit visit, TIntermUnary *node)
multiplierY = mRotationSpecConst->getMultiplierYForDFdy();
}
if (!multiplierX)
{
ASSERT(!multiplierY);
TIntermTyped *flipXY = mDriverUniforms->getFlipXYRef();
TIntermTyped *fragRotation = mDriverUniforms->getFragRotationMatrixRef();
// Get a vec2 with the correct half of ANGLEUniforms.fragRotation
TIntermBinary *halfRotationMat = nullptr;
if (node->getOp() == EOpDFdx)
{
halfRotationMat = new TIntermBinary(EOpIndexDirect, fragRotation, CreateIndexNode(0));
}
else
{
halfRotationMat = new TIntermBinary(EOpIndexDirect, fragRotation, CreateIndexNode(1));
}
// Multiply halfRotationMat by ANGLEUniforms.flipXY and store in a temporary variable
TIntermBinary *rotatedFlipXY = new TIntermBinary(EOpMul, flipXY, halfRotationMat);
const TType *vec2Type = StaticType::GetBasic<EbtFloat, 2>();
TIntermSymbol *tmpRotFlipXY = new TIntermSymbol(CreateTempVariable(mSymbolTable, vec2Type));
TIntermSequence *tmpDecl = new TIntermSequence;
tmpDecl->push_back(CreateTempInitDeclarationNode(&tmpRotFlipXY->variable(), rotatedFlipXY));
insertStatementsInParentBlock(*tmpDecl);
// Get the .x and .y swizzles to use as multipliers
TVector<int> swizzleOffsetX = {0};
TVector<int> swizzleOffsetY = {1};
multiplierX = new TIntermSwizzle(tmpRotFlipXY, swizzleOffsetX);
multiplierY = new TIntermSwizzle(tmpRotFlipXY->deepCopy(), swizzleOffsetY);
}
// Get the results of dFdx(operand) and dFdy(operand), and multiply them by the swizzles
TIntermTyped *operand = node->getOperand();
TIntermUnary *dFdx = new TIntermUnary(EOpDFdx, operand->deepCopy(), node->getFunction());
......@@ -195,11 +156,6 @@ bool Traverser::visitUnaryWithoutRotation(Visit visit, TIntermUnary *node)
TOperator multiplyOp = (objectSize == 1) ? EOpMul : EOpVectorTimesScalar;
TIntermTyped *flipY = mRotationSpecConst->getFlipY();
if (!flipY)
{
TIntermTyped *flipXY = mDriverUniforms->getFlipXYRef();
flipY = new TIntermBinary(EOpIndexDirect, flipXY, CreateIndexNode(1));
}
// Correct dFdy()'s value:
// (dFdy() * mFlipXY.y)
......@@ -217,15 +173,13 @@ bool RewriteDfdy(TCompiler *compiler,
TIntermNode *root,
const TSymbolTable &symbolTable,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
{
// dFdy is only valid in GLSL 3.0 and later.
if (shaderVersion < 300)
return true;
return Traverser::Apply(compiler, compileOptions, root, symbolTable, rotationSpecConst,
driverUniforms);
return Traverser::Apply(compiler, compileOptions, root, symbolTable, rotationSpecConst);
}
} // namespace sh
......@@ -35,8 +35,7 @@ ANGLE_NO_DISCARD bool RewriteDfdy(TCompiler *compiler,
TIntermNode *root,
const TSymbolTable &symbolTable,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
} // namespace sh
......
......@@ -11,7 +11,6 @@
#include "common/angleutils.h"
#include "compiler/translator/StaticType.h"
#include "compiler/translator/SymbolTable.h"
#include "compiler/translator/TranslatorVulkan.h"
#include "compiler/translator/tree_util/DriverUniform.h"
#include "compiler/translator/tree_util/FlipRotateSpecConst.h"
#include "compiler/translator/tree_util/IntermNode_util.h"
......@@ -31,34 +30,29 @@ class Traverser : public TIntermTraverser
TIntermNode *root,
const TSymbolTable &symbolTable,
int ShaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
private:
Traverser(TSymbolTable *symbolTable,
ShCompileOptions compileOptions,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
bool visitAggregate(Visit visit, TIntermAggregate *node) override;
const TSymbolTable *symbolTable = nullptr;
const int shaderVersion;
FlipRotateSpecConst *mRotationSpecConst = nullptr;
const DriverUniform *mDriverUniforms = nullptr;
bool mUsePreRotation = false;
};
Traverser::Traverser(TSymbolTable *symbolTable,
ShCompileOptions compileOptions,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
: TIntermTraverser(true, false, false, symbolTable),
symbolTable(symbolTable),
shaderVersion(shaderVersion),
mRotationSpecConst(rotationSpecConst),
mDriverUniforms(driverUniforms),
mUsePreRotation((compileOptions & SH_ADD_PRE_ROTATION) != 0)
{}
......@@ -68,12 +62,10 @@ bool Traverser::Apply(TCompiler *compiler,
TIntermNode *root,
const TSymbolTable &symbolTable,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
{
TSymbolTable *pSymbolTable = const_cast<TSymbolTable *>(&symbolTable);
Traverser traverser(pSymbolTable, compileOptions, shaderVersion, rotationSpecConst,
driverUniforms);
Traverser traverser(pSymbolTable, compileOptions, shaderVersion, rotationSpecConst);
root->traverse(&traverser);
return traverser.updateTree(compiler, root);
}
......@@ -103,25 +95,8 @@ bool Traverser::visitAggregate(Visit visit, TIntermAggregate *node)
ASSERT(offsetNode->getType() == *(StaticType::GetBasic<EbtFloat, 2>()));
// If pre-rotation is enabled apply the transformation else just flip the Y-coordinate
TIntermTyped *rotatedXY;
if (mUsePreRotation)
{
rotatedXY = mRotationSpecConst->getFragRotationMultiplyFlipXY();
if (!rotatedXY)
{
TIntermTyped *flipXY = mDriverUniforms->getFlipXYRef();
TIntermTyped *fragRotation = mDriverUniforms->getFragRotationMatrixRef();
rotatedXY = new TIntermBinary(EOpMatrixTimesVector, fragRotation, flipXY);
}
}
else
{
rotatedXY = mRotationSpecConst->getFlipXY();
if (!rotatedXY)
{
rotatedXY = mDriverUniforms->getFlipXYRef();
}
}
TIntermTyped *rotatedXY = mUsePreRotation ? mRotationSpecConst->getFragRotationMultiplyFlipXY()
: mRotationSpecConst->getFlipXY();
TIntermBinary *correctedOffset = new TIntermBinary(EOpMul, offsetNode, rotatedXY);
correctedOffset->setLine(offsetNode->getLine());
......@@ -144,8 +119,7 @@ bool RewriteInterpolateAtOffset(TCompiler *compiler,
TIntermNode *root,
const TSymbolTable &symbolTable,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms)
FlipRotateSpecConst *rotationSpecConst)
{
// interpolateAtOffset is only valid in GLSL 3.0 and later.
if (shaderVersion < 300)
......@@ -154,7 +128,7 @@ bool RewriteInterpolateAtOffset(TCompiler *compiler,
}
return Traverser::Apply(compiler, compileOptions, root, symbolTable, shaderVersion,
rotationSpecConst, driverUniforms);
rotationSpecConst);
}
} // namespace sh
......@@ -31,8 +31,7 @@ ANGLE_NO_DISCARD bool RewriteInterpolateAtOffset(TCompiler *compiler,
TIntermNode *root,
const TSymbolTable &symbolTable,
int shaderVersion,
FlipRotateSpecConst *rotationSpecConst,
const DriverUniform *driverUniforms);
FlipRotateSpecConst *rotationSpecConst);
} // namespace sh
......
......@@ -26,22 +26,17 @@ constexpr ImmutableString kEmulatedDepthRangeParams = ImmutableString("ANGLEDept
constexpr const char kViewport[] = "viewport";
constexpr const char kHalfRenderArea[] = "halfRenderArea";
constexpr const char kFlipXY[] = "flipXY";
constexpr const char kNegFlipXY[] = "negFlipXY";
constexpr const char kClipDistancesEnabled[] = "clipDistancesEnabled";
constexpr const char kXfbActiveUnpaused[] = "xfbActiveUnpaused";
constexpr const char kXfbVerticesPerDraw[] = "xfbVerticesPerDraw";
constexpr const char kXfbBufferOffsets[] = "xfbBufferOffsets";
constexpr const char kAcbBufferOffsets[] = "acbBufferOffsets";
constexpr const char kDepthRange[] = "depthRange";
constexpr const char kPreRotation[] = "preRotation";
constexpr const char kFragRotation[] = "fragRotation";
constexpr size_t kNumGraphicsDriverUniforms = 12;
constexpr size_t kNumGraphicsDriverUniforms = 8;
constexpr std::array<const char *, kNumGraphicsDriverUniforms> kGraphicsDriverUniformNames = {
{kViewport, kHalfRenderArea, kFlipXY, kNegFlipXY, kClipDistancesEnabled, kXfbActiveUnpaused,
kXfbVerticesPerDraw, kXfbBufferOffsets, kAcbBufferOffsets, kDepthRange, kPreRotation,
kFragRotation}};
{kViewport, kHalfRenderArea, kClipDistancesEnabled, kXfbActiveUnpaused, kXfbVerticesPerDraw,
kXfbBufferOffsets, kAcbBufferOffsets, kDepthRange}};
constexpr size_t kNumComputeDriverUniforms = 1;
constexpr std::array<const char *, kNumComputeDriverUniforms> kComputeDriverUniformNames = {
......@@ -80,21 +75,12 @@ TFieldList *DriverUniform::createUniformFields(TSymbolTable *symbolTable) const
// This field list mirrors the structure of GraphicsDriverUniforms in ContextVk.cpp.
TFieldList *driverFieldList = new TFieldList;
const std::array<TType *, kNumGraphicsDriverUniforms> kDriverUniformTypes = {{
new TType(EbtFloat, 4),
new TType(EbtFloat, 2),
new TType(EbtFloat, 2),
new TType(EbtFloat, 2),
new TType(EbtUInt), // uint clipDistancesEnabled; // 32 bits for 32 clip distances max
new TType(EbtUInt),
new TType(EbtUInt),
// NOTE: There's a vec3 gap here that can be used in the future
new TType(EbtInt, 4),
new TType(EbtUInt, 4),
createEmulatedDepthRangeType(symbolTable),
new TType(EbtFloat, 2, 2),
new TType(EbtFloat, 2, 2),
}};
const std::array<TType *, kNumGraphicsDriverUniforms> kDriverUniformTypes = {
{new TType(EbtFloat, 4), new TType(EbtFloat, 2),
new TType(EbtUInt), // uint clipDistancesEnabled; // 32 bits for 32 clip distances max
new TType(EbtUInt), new TType(EbtUInt),
// NOTE: There's a vec3 gap here that can be used in the future
new TType(EbtInt, 4), new TType(EbtUInt, 4), createEmulatedDepthRangeType(symbolTable)}};
for (size_t uniformIndex = 0; uniformIndex < kNumGraphicsDriverUniforms; ++uniformIndex)
{
......@@ -173,26 +159,6 @@ TIntermBinary *DriverUniform::createDriverUniformRef(const char *fieldName) cons
return new TIntermBinary(EOpIndexDirectInterfaceBlock, angleUniformsRef, indexRef);
}
TIntermBinary *DriverUniform::getFlipXYRef() const
{
return createDriverUniformRef(kFlipXY);
}
TIntermBinary *DriverUniform::getNegFlipXYRef() const
{
return createDriverUniformRef(kNegFlipXY);
}
TIntermBinary *DriverUniform::getFragRotationMatrixRef() const
{
return createDriverUniformRef(kFragRotation);
}
TIntermBinary *DriverUniform::getPreRotationMatrixRef() const
{
return createDriverUniformRef(kPreRotation);
}
TIntermBinary *DriverUniform::getViewportRef() const
{
return createDriverUniformRef(kViewport);
......@@ -218,15 +184,6 @@ TIntermBinary *DriverUniform::getDepthRangeRef() const
return createDriverUniformRef(kDepthRange);
}
TIntermSwizzle *DriverUniform::getNegFlipYRef() const
{
// Create a swizzle to "negFlipXY.y"
TIntermBinary *negFlipXY = createDriverUniformRef(kNegFlipXY);
TVector<int> swizzleOffsetY = {1};
TIntermSwizzle *negFlipY = new TIntermSwizzle(negFlipXY, swizzleOffsetY);
return negFlipY;
}
TIntermBinary *DriverUniform::getDepthRangeReservedFieldRef() const
{
TIntermBinary *depthRange = createDriverUniformRef(kDepthRange);
......
......@@ -32,15 +32,10 @@ class DriverUniform
bool addComputeDriverUniformsToShader(TIntermBlock *root, TSymbolTable *symbolTable);
bool addGraphicsDriverUniformsToShader(TIntermBlock *root, TSymbolTable *symbolTable);
TIntermBinary *getFlipXYRef() const;
TIntermBinary *getNegFlipXYRef() const;
TIntermBinary *getFragRotationMatrixRef() const;
TIntermBinary *getPreRotationMatrixRef() const;
TIntermBinary *getViewportRef() const;
TIntermBinary *getHalfRenderAreaRef() const;
TIntermBinary *getAbcBufferOffsets() const;
TIntermBinary *getClipDistancesEnabled() const;
TIntermSwizzle *getNegFlipYRef() const;
TIntermBinary *getDepthRangeRef() const;
TIntermBinary *getDepthRangeReservedFieldRef() const;
......
......@@ -32,6 +32,7 @@ using Mat2x2 = std::array<float, 4>;
using Mat2x2EnumMap =
angle::PackedEnumMap<vk::SurfaceRotation, Mat2x2, angle::EnumSize<vk::SurfaceRotation>()>;
// Used to pre-rotate gl_Position for swapchain images on Android.
constexpr Mat2x2EnumMap kPreRotationMatrices = {
{{vk::SurfaceRotation::Identity, {{1.0f, 0.0f, 0.0f, 1.0f}}},
{vk::SurfaceRotation::Rotated90Degrees, {{0.0f, -1.0f, 1.0f, 0.0f}}},
......@@ -42,6 +43,7 @@ constexpr Mat2x2EnumMap kPreRotationMatrices = {
{vk::SurfaceRotation::FlippedRotated180Degrees, {{-1.0f, 0.0f, 0.0f, -1.0f}}},
{vk::SurfaceRotation::FlippedRotated270Degrees, {{0.0f, 1.0f, -1.0f, 0.0f}}}}};
// Used to pre-rotate gl_FragCoord for swapchain images on Android.
constexpr Mat2x2EnumMap kFragRotationMatrices = {
{{vk::SurfaceRotation::Identity, {{1.0f, 0.0f, 0.0f, 1.0f}}},
{vk::SurfaceRotation::Rotated90Degrees, {{0.0f, 1.0f, 1.0f, 0.0f}}},
......@@ -90,6 +92,12 @@ TIntermTyped *GenerateMat2x2ArrayWithIndex(const Mat2x2EnumMap &matrix, TIntermS
using Vec2 = std::array<float, 2>;
using Vec2EnumMap =
angle::PackedEnumMap<vk::SurfaceRotation, Vec2, angle::EnumSize<vk::SurfaceRotation>()>;
// Y-axis flipping only comes into play with the default framebuffer (i.e. a swapchain image).
// For 0-degree rotation, an FBO or pbuffer could be the draw framebuffer, and so we must check
// whether flipY should be positive or negative. All other rotations, will be to the default
// framebuffer, and so the value of isViewportFlipEnabledForDrawFBO() is assumed true; the
// appropriate flipY value is chosen such that gl_FragCoord is positioned at the lower-left
// corner of the window.
constexpr Vec2EnumMap kFlipXYValue = {
{{vk::SurfaceRotation::Identity, {{1.0f, 1.0f}}},
{vk::SurfaceRotation::Rotated90Degrees, {{1.0f, 1.0f}}},
......
......@@ -24,7 +24,7 @@ class FlipRotateSpecConst
{
public:
FlipRotateSpecConst();
~FlipRotateSpecConst();
virtual ~FlipRotateSpecConst();
TIntermTyped *getMultiplierXForDFdx();
TIntermTyped *getMultiplierYForDFdx();
......@@ -32,14 +32,16 @@ class FlipRotateSpecConst
TIntermTyped *getMultiplierYForDFdy();
TIntermTyped *getPreRotationMatrix();
TIntermTyped *getFragRotationMatrix();
TIntermTyped *getFlipXY();
TIntermTyped *getNegFlipXY();
TIntermTyped *getFlipY();
TIntermTyped *getNegFlipY();
TIntermTyped *getFragRotationMultiplyFlipXY();
void generateSymbol(TSymbolTable *symbolTable);
void outputLayoutString(TInfoSinkBase &sink) const;
// Let Metal override until they provide specialized constant support
virtual TIntermTyped *getFlipXY();
virtual TIntermTyped *getNegFlipXY();
virtual TIntermTyped *getFlipY();
virtual TIntermTyped *getNegFlipY();
virtual void generateSymbol(TSymbolTable *symbolTable);
virtual void outputLayoutString(TInfoSinkBase &sink) const;
private:
TIntermSymbol *mSpecConstSymbol;
......
......@@ -479,8 +479,6 @@ class ContextMtl : public ContextImpl, public mtl::Context
float viewport[4];
float halfRenderArea[2];
float flipXY[2];
float negFlipXY[2];
// 32 bits for 32 clip distances
uint32_t enabledClipDistances;
......@@ -496,16 +494,8 @@ class ContextMtl : public ContextImpl, public mtl::Context
// We'll use x, y, z, w for near / far / diff / zscale respectively.
float depthRange[4];
// Used to pre-rotate gl_Position for Vulkan swapchain images on Android (a mat2, which is
// padded to the size of two vec4's).
// Unused in Metal.
float preRotation[8] = {};
// Used to pre-rotate gl_FragCoord for Vulkan swapchain images on Android (a mat2, which is
// padded to the size of two vec4's).
// Unused in Metal.
float fragRotation[8] = {};
float flipXY[2];
float negFlipXY[2];
uint32_t coverageMask;
float padding2[3];
......
......@@ -2092,10 +2092,6 @@ angle::Result ContextMtl::handleDirtyDriverUniforms(const gl::Context *context,
static_cast<float>(mDrawFramebuffer->getState().getDimensions().width) * 0.5f;
mDriverUniforms.halfRenderArea[1] =
static_cast<float>(mDrawFramebuffer->getState().getDimensions().height) * 0.5f;
mDriverUniforms.flipXY[0] = 1.0f;
mDriverUniforms.flipXY[1] = mDrawFramebuffer->flipY() ? -1.0f : 1.0f;
mDriverUniforms.negFlipXY[0] = mDriverUniforms.flipXY[0];
mDriverUniforms.negFlipXY[1] = -mDriverUniforms.flipXY[1];
// gl_ClipDistance
mDriverUniforms.enabledClipDistances = mState.getEnabledClipDistances().bits();
......@@ -2105,7 +2101,10 @@ angle::Result ContextMtl::handleDirtyDriverUniforms(const gl::Context *context,
mDriverUniforms.depthRange[2] = depthRangeDiff;
mDriverUniforms.depthRange[3] = NeedToInvertDepthRange(depthRangeNear, depthRangeFar) ? -1 : 1;
// NOTE(hqle): preRotation & fragRotation are unused.
mDriverUniforms.flipXY[0] = 1.0f;
mDriverUniforms.flipXY[1] = mDrawFramebuffer->flipY() ? -1.0f : 1.0f;
mDriverUniforms.negFlipXY[0] = mDriverUniforms.flipXY[0];
mDriverUniforms.negFlipXY[1] = -mDriverUniforms.flipXY[1];
// Sample coverage mask
uint32_t sampleBitCount = mDrawFramebuffer->getSamples();
......
......@@ -15,6 +15,7 @@
#include <limits>
#include <map>
#include "GLSLANG/ShaderLang.h"
#include "common/angleutils.h"
#include "common/utilities.h"
#include "libANGLE/angletypes.h"
......@@ -44,20 +45,21 @@ namespace rx
class ContextImpl;
// The possible rotations of the surface/draw framebuffer, particularly for the Vulkan back-end on
// Android.
enum class SurfaceRotation
// Android. This is duplicate of ShaderLang.h declaration to avoid having to litter the renderer
// code with sh::vk namespace string.
enum class SurfaceRotation : uint32_t
{
Identity,
Rotated90Degrees,
Rotated180Degrees,
Rotated270Degrees,
FlippedIdentity,
FlippedRotated90Degrees,
FlippedRotated180Degrees,
FlippedRotated270Degrees,
InvalidEnum,
EnumCount = InvalidEnum,
Identity = ToUnderlying(sh::vk::SurfaceRotation::Identity),
Rotated90Degrees = ToUnderlying(sh::vk::SurfaceRotation::Rotated90Degrees),
Rotated180Degrees = ToUnderlying(sh::vk::SurfaceRotation::Rotated180Degrees),
Rotated270Degrees = ToUnderlying(sh::vk::SurfaceRotation::Rotated270Degrees),
FlippedIdentity = ToUnderlying(sh::vk::SurfaceRotation::FlippedIdentity),
FlippedRotated90Degrees = ToUnderlying(sh::vk::SurfaceRotation::FlippedRotated90Degrees),
FlippedRotated180Degrees = ToUnderlying(sh::vk::SurfaceRotation::FlippedRotated180Degrees),
FlippedRotated270Degrees = ToUnderlying(sh::vk::SurfaceRotation::FlippedRotated270Degrees),
InvalidEnum = ToUnderlying(sh::vk::SurfaceRotation::InvalidEnum),
EnumCount = InvalidEnum,
};
void RotateRectangle(const SurfaceRotation rotation,
......
......@@ -62,8 +62,6 @@ struct GraphicsDriverUniforms
// Used to flip gl_FragCoord (both .xy for Android pre-rotation; only .y for desktop)
std::array<float, 2> halfRenderArea;
std::array<float, 2> flipXY;
std::array<float, 2> negFlipXY;
// 32 bits for 32 clip planes
uint32_t enabledClipPlanes;
......@@ -83,13 +81,6 @@ struct GraphicsDriverUniforms
// We'll use x, y, z for near / far / diff respectively.
std::array<float, 4> depthRange;
// Used to pre-rotate gl_Position for swapchain images on Android (a mat2, which is padded to
// the size of two vec4's).
std::array<float, 8> preRotation;
// Used to pre-rotate gl_FragCoord for swapchain images on Android (a mat2, which is padded to
// the size of two vec4's).
std::array<float, 8> fragRotation;
};
struct ComputeDriverUniforms
......@@ -179,51 +170,6 @@ bool IsRenderPassStartedAndUsesImage(const vk::CommandBufferHelper &renderPassCo
return renderPassCommands.started() && renderPassCommands.usesImageInRenderPass(image);
}
// When an Android surface is rotated differently than the device's native orientation, ANGLE must
// rotate gl_Position in the vertex shader and gl_FragCoord in the fragment shader. The following
// are the rotation matrices used.
//
// Note: these are mat2's that are appropriately padded (4 floats per row).
using PreRotationMatrixValues = std::array<float, 8>;
constexpr angle::PackedEnumMap<rx::SurfaceRotation,
PreRotationMatrixValues,
angle::EnumSize<rx::SurfaceRotation>()>
kPreRotationMatrices = {
{{rx::SurfaceRotation::Identity, {{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated90Degrees,
{{0.0f, -1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated180Degrees,
{{-1.0f, 0.0f, 0.0f, 0.0f, 0.0f, -1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated270Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, -1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedIdentity,
{{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, -1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated90Degrees,
{{0.0f, -1.0f, 0.0f, 0.0f, -1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated180Degrees,
{{-1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated270Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}}}};
constexpr angle::PackedEnumMap<rx::SurfaceRotation,
PreRotationMatrixValues,
angle::EnumSize<rx::SurfaceRotation>()>
kFragRotationMatrices = {
{{rx::SurfaceRotation::Identity, {{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated90Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated180Degrees,
{{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::Rotated270Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedIdentity, {{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated90Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated180Degrees,
{{1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f}}},
{rx::SurfaceRotation::FlippedRotated270Degrees,
{{0.0f, 1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f}}}}};
bool IsRotatedAspectRatio(SurfaceRotation rotation)
{
return ((rotation == SurfaceRotation::Rotated90Degrees) ||
......@@ -3592,46 +3538,11 @@ angle::Result ContextVk::handleDirtyGraphicsDriverUniforms(const gl::Context *co
std::swap(glViewport.x, glViewport.y);
std::swap(glViewport.width, glViewport.height);
}
float flipX = 1.0f;
float flipY = -1.0f;
// Y-axis flipping only comes into play with the default framebuffer (i.e. a swapchain image).
// For 0-degree rotation, an FBO or pbuffer could be the draw framebuffer, and so we must check
// whether flipY should be positive or negative. All other rotations, will be to the default
// framebuffer, and so the value of isViewportFlipEnabledForDrawFBO() is assumed true; the
// appropriate flipY value is chosen such that gl_FragCoord is positioned at the lower-left
// corner of the window.
switch (mCurrentRotationDrawFramebuffer)
{
case SurfaceRotation::Identity:
flipX = 1.0f;
flipY = isViewportFlipEnabledForDrawFBO() ? -1.0f : 1.0f;
break;
case SurfaceRotation::Rotated90Degrees:
ASSERT(isViewportFlipEnabledForDrawFBO());
flipX = 1.0f;
flipY = 1.0f;
std::swap(halfRenderAreaWidth, halfRenderAreaHeight);
break;
case SurfaceRotation::Rotated180Degrees:
ASSERT(isViewportFlipEnabledForDrawFBO());
flipX = -1.0f;
flipY = 1.0f;
break;
case SurfaceRotation::Rotated270Degrees:
ASSERT(isViewportFlipEnabledForDrawFBO());
flipX = -1.0f;
flipY = -1.0f;
break;
default:
UNREACHABLE();
break;
}
uint32_t xfbActiveUnpaused = mState.isTransformFeedbackActiveUnpaused();
float depthRangeNear = mState.getNearPlane();
float depthRangeFar = mState.getFarPlane();
float depthRangeDiff = depthRangeFar - depthRangeNear;
float depthRangeNear = mState.getNearPlane();
float depthRangeFar = mState.getFarPlane();
float depthRangeDiff = depthRangeFar - depthRangeNear;
// Copy and flush to the device.
GraphicsDriverUniforms *driverUniforms = reinterpret_cast<GraphicsDriverUniforms *>(ptr);
......@@ -3639,21 +3550,13 @@ angle::Result ContextVk::handleDirtyGraphicsDriverUniforms(const gl::Context *co
{static_cast<float>(glViewport.x), static_cast<float>(glViewport.y),
static_cast<float>(glViewport.width), static_cast<float>(glViewport.height)},
{halfRenderAreaWidth, halfRenderAreaHeight},
{flipX, flipY},
{flipX, -flipY},
mState.getEnabledClipDistances().bits(),
xfbActiveUnpaused,
mXfbVertexCountPerInstance,
{},
{},
{},
{depthRangeNear, depthRangeFar, depthRangeDiff, 0.0f},
{},
{}};
memcpy(&driverUniforms->preRotation, &kPreRotationMatrices[mCurrentRotationDrawFramebuffer],
sizeof(PreRotationMatrixValues));
memcpy(&driverUniforms->fragRotation, &kFragRotationMatrices[mCurrentRotationDrawFramebuffer],
sizeof(PreRotationMatrixValues));
{depthRangeNear, depthRangeFar, depthRangeDiff, 0.0f}};
if (xfbActiveUnpaused)
{
......
......@@ -72,9 +72,6 @@ std::shared_ptr<WaitableCompileEvent> ShaderVk::compile(const gl::Context *conte
// context state does not allow it
compileOptions |= SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION;
// Let compiler use specialized constant for pre-rotation.
compileOptions |= SH_USE_ROTATION_SPECIALIZATION_CONSTANT;
if (contextVk->getFeatures().enablePreRotateSurfaces.enabled ||
contextVk->getFeatures().emulatedPrerotation90.enabled ||
contextVk->getFeatures().emulatedPrerotation180.enabled ||
......
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