Provides multiple implementations of Android's SkBitmapRegionDecoder

src/utils/SkBitmapRegionCanvas.h

+/*
+ * Copyright (C) 2015 The Android Open Source Project

[scroggo, 2015/08/13 16:53:07]

This is the wrong header. If this is new code in Skia, I think you want the one
used in the other files?

Is this a modified version of a file that is in Android?

[msarett, 2015/08/13 18:10:23]

You're right.  BitmapRegionSampler needs the Android header I think, but this
should have the regular skia header.

+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "SkBitmap.h"
+#include "SkBitmapRegionDecoder.h"
+#include "SkScanlineDecoder.h"
+
+/*
+ * This class implements SkBitmapRegionDecoder using SkCanvas.

[scroggo, 2015/08/13 16:53:07]

by drawing to an SkCanvas?

[msarett, 2015/08/13 18:10:23]

Done.

+ */
+class SkBitmapRegionCanvas : public SkBitmapRegionDecoder {
+public:
+
+    /*
+     * This has several key differences from the Android version:

[scroggo, 2015/08/13 16:53:07]

I think it's probably good to contrast with the android version, but it would be
helpful to explain what the parameters mean.

(That said, maybe that is in the base class description, which I should look at
to understand this file?)

[msarett, 2015/08/13 18:10:23]

Added comments on the parameters to the base class.

+     *     Returns a Skia bitmap instead of an Android bitmap.
+     *     Android version attempts to reuse a recycled bitmap.
+     */
+    SkBitmap* decodeRegion(int start_x, int start_y, int width, int height,

[scroggo, 2015/08/13 16:53:07]

Out of curiosity, why does this return a pointer to an SkBitmap? It seems like
it could return an SkBitmap (its operator= method is relatively cheap) or take a
pointer to an SkBitmap as a parameter?

Ah, I see that you return NULL on failure. The latter method could also return a
bool, like SkImageDecoder does.

[msarett, 2015/08/13 18:10:23]

The reason this returns an SkBitmap* is that I was going for the closest match
to the Android API.

decodeRegion (Java) calls nativeDecodeRegion (C++) which essentially returns a
pointer to an Android bitmap (which I think is basically an SkBitmap).  Maybe
it's not so important to match this?  It seems logical to me - especially since
we will be moving one of these implementations into Android.

FYI, nativeDecodeRegion (C++) calls SkBitmapRegionDecoder::decodeRegion (C++)
passing in a pointer to the SkBitmap and returns a bool.  I combined these two
functions and matched the nativeDecodeRegion API.

+                           int sampleSize, SkColorType prefColorType) override;
+
+    SkBitmapRegionCanvas(SkScanlineDecoder* decoder);

[scroggo, 2015/08/13 16:53:07]

nit:

We usually put constructors first, I believe. Also, comments explaining
ownership?

[msarett, 2015/08/13 18:10:23]

Done.

+
+    ~SkBitmapRegionCanvas() {}

[scroggo, 2015/08/13 16:53:07]

Is this necessary? I think the compiler will generate this for you?

[msarett, 2015/08/13 18:10:23]

Done.

+
+private:
+
+    SkAutoTDelete<SkScanlineDecoder> fDecoder;
+
+    typedef SkBitmapRegionDecoder INHERITED;
+
+};