Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
194703011:
Add effect-based clip for nine-patch SkRRects. (Closed)
|
Created:
6 years, 9 months ago by bsalomon Modified:
6 years, 9 months ago CC:
skia-review_googlegroups.com Base URL:
https://skia.googlecode.com/svn/trunk Visibility:
Public. |
DescriptionAdd effect-based clip for nine-patch SkRRects.
BUG=skia:2181
Committed: http://code.google.com/p/skia/source/detail?r=13794
Patch Set 1 #
Total comments: 6
Patch Set 2 : rebase #Patch Set 3 : rebase #Patch Set 4 : rebase again #
Messages
Total messages: 13 (0 generated)
Builds upon (and fixes a few nits) in: https://codereview.chromium.org/194603004/
lgtm + comments https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... File src/gpu/effects/GrRRectEffect.cpp (right): https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:382: kNinePatch_RRectType, This might be worth adding to SkRRect. https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:616: uman.set2f(fInvRadiiSqdUniform, 1.f / (r0.fX * r0.fX), Use SkScalarInvert or SkScalarFastInvert? https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:671: cornerFlags = -1; This seems a little hacky, but I don't have a better suggestion.
https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... File src/gpu/effects/GrRRectEffect.cpp (right): https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:382: kNinePatch_RRectType, On 2014/03/12 13:37:55, JimVV wrote: > This might be worth adding to SkRRect. Will do in follow-up CL https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:616: uman.set2f(fInvRadiiSqdUniform, 1.f / (r0.fX * r0.fX), On 2014/03/12 13:37:55, JimVV wrote: > Use SkScalarInvert or SkScalarFastInvert? I checked with Mike and he sez using SkScalarInvert doesn't add any value (now that we no longer support scalar=fixed) https://codereview.chromium.org/194703011/diff/1/src/gpu/effects/GrRRectEffec... src/gpu/effects/GrRRectEffect.cpp:671: cornerFlags = -1; On 2014/03/12 13:37:55, JimVV wrote: > This seems a little hacky, but I don't have a better suggestion. yup :)
The CQ bit was checked by bsalomon@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/bsalomon@google.com/194703011/30001
The CQ bit was unchecked by rmistry@google.com
The CQ bit was checked by rmistry@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/bsalomon@google.com/194703011/30001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for src/gpu/effects/GrRRectEffect.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file src/gpu/effects/GrRRectEffect.cpp
Hunk #5 FAILED at 460.
1 out of 13 hunks FAILED -- saving rejects to file
src/gpu/effects/GrRRectEffect.cpp.rej
Patch: src/gpu/effects/GrRRectEffect.cpp
Index: src/gpu/effects/GrRRectEffect.cpp
diff --git a/src/gpu/effects/GrRRectEffect.cpp
b/src/gpu/effects/GrRRectEffect.cpp
index
636730db221b62cc291dcb70c8c38e94424ca40a..324d02da02df5c140aec42a9a98c5e0607535cc5
100644
--- a/src/gpu/effects/GrRRectEffect.cpp
+++ b/src/gpu/effects/GrRRectEffect.cpp
@@ -375,22 +375,27 @@ void GLCircularRRectEffect::setData(const
GrGLUniformManager& uman,
class GLEllipticalRRectEffect;
-/**
- * Currently this effect only supports "simple" elliptical round rects (i.e.
- * the corners all have a common x/y radii pair).
- */
class EllipticalRRectEffect : public GrEffect {
public:
- // This effect only supports rrects where the radii are >= kRadiusMin
+ // This effect currently works for these two classifications of SkRRects
+ enum RRectType {
+ kSimple_RRectType, // SkRRect::kSimple_Type
+ kNinePatch_RRectType, // The two left x radii are the same, the two
+ // top y radii are the same, etc.
+ };
+
+ // This effect only supports rrects where the radii are >= kRadiusMin.
static const SkScalar kRadiusMin;
- static GrEffectRef* Create(GrEffectEdgeType, const SkRRect&);
+ static GrEffectRef* Create(GrEffectEdgeType, RRectType, const SkRRect&);
virtual ~EllipticalRRectEffect() {};
static const char* Name() { return "EllipticalRRect"; }
const SkRRect& getRRect() const { return fRRect; }
+ RRectType getRRectType() const { return fRRectType; }
+
GrEffectEdgeType getEdgeType() const { return fEdgeType; }
typedef GLEllipticalRRectEffect GLEffect;
@@ -400,11 +405,12 @@ public:
virtual const GrBackendEffectFactory& getFactory() const SK_OVERRIDE;
private:
- EllipticalRRectEffect(GrEffectEdgeType, const SkRRect&);
+ EllipticalRRectEffect(GrEffectEdgeType, RRectType, const SkRRect&);
virtual bool onIsEqual(const GrEffect& other) const SK_OVERRIDE;
SkRRect fRRect;
+ RRectType fRRectType;
GrEffectEdgeType fEdgeType;
GR_DECLARE_EFFECT_TEST;
@@ -415,9 +421,11 @@ private:
const SkScalar EllipticalRRectEffect::kRadiusMin = 0.5f;
GrEffectRef* EllipticalRRectEffect::Create(GrEffectEdgeType edgeType,
+ RRectType rrType,
const SkRRect& rrect) {
-// SkASSERT(kFillAA_GrEffectEdgeType == edgeType ||
kInverseFillAA_GrEffectEdgeType == edgeType);
- return CreateEffectRef(AutoEffectUnref(SkNEW_ARGS(EllipticalRRectEffect,
(edgeType, rrect))));
+ SkASSERT(kFillAA_GrEffectEdgeType == edgeType ||
kInverseFillAA_GrEffectEdgeType == edgeType);
+ return CreateEffectRef(AutoEffectUnref(SkNEW_ARGS(EllipticalRRectEffect,
(edgeType, rrType,
+
rrect))));
}
void EllipticalRRectEffect::getConstantColorComponents(GrColor* color,
uint32_t* validFlags) const {
@@ -428,14 +436,17 @@ const GrBackendEffectFactory&
EllipticalRRectEffect::getFactory() const {
return GrTBackendEffectFactory<EllipticalRRectEffect>::getInstance();
}
-EllipticalRRectEffect::EllipticalRRectEffect(GrEffectEdgeType edgeType, const
SkRRect& rrect)
+EllipticalRRectEffect::EllipticalRRectEffect(GrEffectEdgeType edgeType,
RRectType rrType,
+ const SkRRect& rrect)
: fRRect(rrect)
+ , fRRectType(rrType)
, fEdgeType(edgeType){
this->setWillReadFragmentPosition();
}
bool EllipticalRRectEffect::onIsEqual(const GrEffect& other) const {
const EllipticalRRectEffect& erre =
CastEffect<EllipticalRRectEffect>(other);
+ // No need to check fRRectType as it is derived from fRRect.
return fEdgeType == erre.fEdgeType && fRRect == erre.fRRect;
}
@@ -449,15 +460,34 @@ GrEffectRef* EllipticalRRectEffect::TestCreate(SkRandom*
random,
GrTexture*[]) {
SkScalar w = random->nextRangeScalar(20.f, 1000.f);
SkScalar h = random->nextRangeScalar(20.f, 1000.f);
- SkScalar rx = random->nextRangeF(kRadiusMin, 9.f);
- SkScalar ry = random->nextRangeF(kRadiusMin, 9.f);
+ SkVector r[4];
+ r[SkRRect::kUpperLeft_Corner].fX = random->nextRangeF(kRadiusMin, 9.f);
+ // ensure at least one corner really is elliptical
+ do {
+ r[SkRRect::kUpperLeft_Corner].fY = random->nextRangeF(kRadiusMin, 9.f);
+ } while (r[SkRRect::kUpperLeft_Corner].fY ==
r[SkRRect::kUpperLeft_Corner].fX);
+
SkRRect rrect;
- rrect.setRectXY(SkRect::MakeWH(w, h), rx, ry);
+ if (random->nextBool()) {
+ // half the time create a four-radii rrect.
+ r[SkRRect::kLowerRight_Corner].fX = random->nextRangeF(kRadiusMin,
9.f);
+ r[SkRRect::kLowerRight_Corner].fY = random->nextRangeF(kRadiusMin,
9.f);
+
+ r[SkRRect::kUpperRight_Corner].fX = r[SkRRect::kLowerRight_Corner].fX;
+ r[SkRRect::kUpperRight_Corner].fY = r[SkRRect::kUpperLeft_Corner].fY;
+
+ r[SkRRect::kLowerLeft_Corner].fX = r[SkRRect::kUpperLeft_Corner].fX;
+ r[SkRRect::kLowerLeft_Corner].fY = r[SkRRect::kLowerRight_Corner].fY;
+
+ rrect.setRectRadii(SkRect::MakeWH(w, h), r);
+ } else {
+ rrect.setRectXY(SkRect::MakeWH(w, h), r[SkRRect::kUpperLeft_Corner].fX,
+
r[SkRRect::kUpperLeft_Corner].fY);
+ }
GrEffectRef* effect;
do {
- GrEffectEdgeType et = random->nextBool() ? kFillAA_GrEffectEdgeType :
-
kInverseFillAA_GrEffectEdgeType;
- effect = EllipticalRRectEffect::Create(et, rrect);
+ GrEffectEdgeType et =
(GrEffectEdgeType)random->nextULessThan(kGrEffectEdgeTypeCnt);
+ effect = GrRRectEffect::Create(et, rrect);
} while (NULL == effect);
return effect;
}
@@ -482,7 +512,7 @@ public:
private:
GrGLUniformManager::UniformHandle fInnerRectUniform;
- GrGLUniformManager::UniformHandle fInvRadiusXYSqdUniform;
+ GrGLUniformManager::UniformHandle fInvRadiiSqdUniform;
SkRRect fPrevRRect;
typedef GrGLEffect INHERITED;
};
@@ -502,16 +532,11 @@ void GLEllipticalRRectEffect::emitCode(GrGLShaderBuilder*
builder,
const TextureSamplerArray& samplers) {
const EllipticalRRectEffect& erre =
drawEffect.castEffect<EllipticalRRectEffect>();
const char *rectName;
- const char *invRadiusXYSqdName;
// The inner rect is the rrect bounds inset by the x/y radii
fInnerRectUniform =
builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
kVec4f_GrSLType,
"innerRect",
&rectName);
- fInvRadiusXYSqdUniform =
builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
- kVec2f_GrSLType,
- "invRadiusXY",
- &invRadiusXYSqdName);
const char* fragmentPos = builder->fragmentPosition();
// At each quarter-ellipse corner we compute a vector that is the offset of
the fragment pos
// to the ellipse center. The vector is pinned in x and y to be in the
quarter-plane relevant
@@ -526,9 +551,33 @@ void GLEllipticalRRectEffect::emitCode(GrGLShaderBuilder*
builder,
// need be computed to determine the min alpha.
builder->fsCodeAppendf("\t\tvec2 dxy0 = %s.xy - %s.xy;\n", rectName,
fragmentPos);
builder->fsCodeAppendf("\t\tvec2 dxy1 = %s.xy - %s.zw;\n", fragmentPos,
rectName);
- builder->fsCodeAppend("\t\tvec2 dxy = max(max(dxy0, dxy1), 0.0);\n");
- // Z is the x/y offsets divided by squared radii.
- builder->fsCodeAppendf("\t\tvec2 Z = dxy * %s;\n", invRadiusXYSqdName);
+ switch (erre.getRRectType()) {
+ case EllipticalRRectEffect::kSimple_RRectType: {
+ const char *invRadiiXYSqdName;
+ fInvRadiiSqdUniform =
builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
+ kVec2f_GrSLType,
+ "invRadiiXY",
+ &invRadiiXYSqdName);
+ builder->fsCodeAppend("\t\tvec2 dxy = max(max(dxy0, dxy1),
0.0);\n");
+ // Z is the x/y offsets divided by squared radii.
+ builder->fsCodeAppendf("\t\tvec2 Z = dxy * %s;\n",
invRadiiXYSqdName);
+ break;
+ }
+ case EllipticalRRectEffect::kNinePatch_RRectType: {
+ const char *invRadiiLTRBSqdName;
+ fInvRadiiSqdUniform =
builder->addUniform(GrGLShaderBuilder::kFragment_Visibility,
+ kVec4f_GrSLType,
+ "invRadiiLTRB",
+ &invRadiiLTRBSqdName);
+ builder->fsCodeAppend("\t\tvec2 dxy = max(max(dxy0, dxy1),
0.0);\n");
+ // Z is the x/y offsets divided by squared radii. We only care
about the (at most) one
+ // corner where both the x and y offsets are positive, hence the
maxes. (The inverse
+ // squared radii will always be positive.)
+ builder->fsCodeAppendf("\t\tvec2 Z = max(max(dxy0 * %s.xy, dxy1 *
%s.zw), 0.0);\n",
+ invRadiiLTRBSqdName, invRadiiLTRBSqdName);
+ break;
+ }
+ }
// implicit is the evaluation of (x/a)^2 + (y/b)^2 …
(message too large)
The CQ bit was checked by bsalomon@google.com
CQ is trying da patch. Follow status at https://skia-tree-status.appspot.com/cq/bsalomon@google.com/194703011/50001
Message was sent while issue was closed.
Change committed as 13794 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
