Move bounds to GrBatch

src/gpu/GrBatch.h

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #ifndef GrBatch_DEFINED
 #define GrBatch_DEFINED
 
@@ skipping 43 matching lines @@
      * initBatchTracker is a hook for the some additional overrides / optimization possibilities
      * from the GrXferProcessor.
      */
     virtual void initBatchTracker(const GrPipelineInfo& init) = 0;
 
     bool combineIfPossible(GrBatch* that) {
         if (this->classID() != that->classID()) {
             return false;
         }
 
-        return onCombineIfPossible(that);
+        return this->onCombineIfPossible(that);
     }
 
     virtual bool onCombineIfPossible(GrBatch*) = 0;
 
     virtual void generateGeometry(GrBatchTarget*, const GrPipeline*) = 0;
 
+    const SkRect& bounds() const { return fBounds; }
+
     // TODO this goes away when batches are everywhere
     void setNumberOfDraws(int numberOfDraws) { fNumberOfDraws = numberOfDraws; }
     int numberOfDraws() const { return fNumberOfDraws; }
 
     void* operator new(size_t size);
     void operator delete(void* target);
 
     void* operator new(size_t size, void* placement) {
         return ::operator new(size, placement);
     }
@@ skipping 15 matching lines @@
     SkDEBUGCODE(bool isUsed() const { return fUsed; })
 
 protected:
     template <typename PROC_SUBCLASS> void initClassID() {
          static uint32_t kClassID = GenClassID();
          fClassID = kClassID;
     }
 
     uint32_t fClassID;
 
+    // Its always better to compute some conservative bounds, but some calls are exceedingly
+    // infrequent or the cost to compute bounds would be extremely expensive
+    void setBoundsLargest() { fBounds.setLargest(); }
+    void setBounds(const SkRect& newBounds) { fBounds = newBounds; }

[robertphillips, 2015/05/01 13:03:00]

Why not "const SkRect&" ?

[joshualitt, 2015/05/01 13:22:28]

not sure I follow, the function signature is "const SkRect&"??

[bsalomon, 2015/05/01 13:24:53]

Rob writes his comments using 0-based indexing. So this command for line 111 is
what you and I would think of as line 112. :)

[joshualitt, 2015/05/01 13:49:58]

lol, okay so, clients need to outset / map this rect.  I could either expose
every rect method which might be used, or always force the client to stack
allocate a rect, and then copy it into the class.  I'd rather not copy, even if
this breaks encapsulation a bit.  Its only exposed to derived classes, so unless
you *REALLY* want me to do the copy, I'd prefer it stay the way it is.

[bsalomon, 2015/05/01 14:29:55]

Does making fBounds protected solve your problem? It seems a bit silly to have a
private data member with a protected non-const ptr getter unless the base class
can relocate the member. We use protected data members often.

+    SkRect* getBounds() { return &fBounds; }
+
+    // TODO will our bounds ever actually be empty?

[bsalomon, 2015/04/30 18:53:23]

probably should never be empty. Why have a batch that we know will affect zero
pixels?

[joshualitt, 2015/05/01 13:49:57]

Well, it looks like the bounds for horizontal / vertical lines show up as empty.

[bsalomon, 2015/05/01 14:29:55]

Oh, duh. I guess we have a different meaning of "empty" here. Here left==right
is allowed, but left>right is not. We need to make sure join works correctly for
our use case when either rect is a line or point.

+    void joinBounds(const SkRect& otherBounds) { return fBounds.join(otherBounds); }
+
 private:
     static uint32_t GenClassID() {
         // fCurrProcessorClassID has been initialized to kIllegalProcessorClassID. The
         // atomic inc returns the old value not the incremented value. So we add
         // 1 to the returned value.
         uint32_t id = static_cast<uint32_t>(sk_atomic_inc(&gCurrBatchClassID)) + 1;
         if (!id) {
             SkFAIL("This should never wrap as it should only be called once for each GrBatch "
                    "subclass.");
         }
         return id;
     }
 
     enum {
         kIllegalBatchClassID = 0,
     };
     static int32_t gCurrBatchClassID;
 
     SkDEBUGCODE(bool fUsed;)
 
     int fNumberOfDraws;
+    SkRect fBounds;
 
     typedef SkRefCnt INHERITED;
 };
 
 #endif