Counterproposal for skirting the BBH when the query fully contains the picture.

src/core/SkRecordDraw.cpp

 /*
  * Copyright 2014 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkRecordDraw.h"
 #include "SkTSort.h"
 
 void SkRecordDraw(const SkRecord& record,
                   SkCanvas* canvas,
                   const SkBBoxHierarchy* bbh,
                   SkDrawPictureCallback* callback) {
     SkAutoCanvasRestore saveRestore(canvas, true /*save now, restore at exit*/);
 
     if (NULL != bbh) {
         // Draw only ops that affect pixels in the canvas's current clip.
         SkIRect query;
 #if 1   // TODO: Why is this the right way to make the query?  I'd think it'd be the else branch.

[reed1, 2014/08/18 19:42:39]

Not sure I follow the comment.
Do we want to keep the #if 1 in the checked-in code?

[mtklein, 2014/08/18 19:45:24]

Yeah, I'd say so, at least until I understand why we build the BBH in
device-coordinates but seemingly make queries in user-space.  As it is, it
happens that GMs draw right this way and wrong in the else case, but I'd like to
leave a better explanation here.

[reed1, 2014/08/18 19:48:54]

Ah. Does the comment mean : "why isn't getClipDeviceBounds sufficient" ?

Lets say something about how to reproduce the problem here in the code if we're
going to have #else.

e.g.
/*
 * Seems like getClipDeviceBounds should be sufficient, but if we call it, some
GMs fail to draw correctly.
 * Lets figure out why : is that a bug in the GM, a bug in BBH, a bug in
getClipDeviceBounds, or other.
 */

-        SkRect clipBounds;
-        canvas->getClipBounds(&clipBounds);
+        SkRect clipBounds = { 0, 0, 0, 0 };
+        (void)canvas->getClipBounds(&clipBounds);
         clipBounds.roundOut(&query);
 #else
         canvas->getClipDeviceBounds(&query);
 #endif
         SkTDArray<void*> ops;
         bbh->search(query, &ops);
 
         SkRecords::Draw draw(canvas);
         for (int i = 0; i < ops.count(); i++) {
             if (NULL != callback && callback->abortDrawing()) {
@@ skipping 347 matching lines @@
     // Used to track the bounds of Save/Restore blocks and the control ops inside them.
     SkTDArray<SaveBounds> fSaveStack;
     SkTDArray<unsigned>   fControlIndices;
 };
 
 }  // namespace SkRecords
 
 void SkRecordFillBounds(const SkRecord& record, SkBBoxHierarchy* bbh) {
     SkRecords::FillBounds(record, bbh);
 }