From 719dda52e57e979cc819e4531cd1537588a724dd Mon Sep 17 00:00:00 2001 From: meetwq Date: Fri, 30 Aug 2024 22:53:22 +0800 Subject: [PATCH 1/5] add missing pybind11 include There are arguments of type `std::vector` in `Point.Offset` (located in the file `src/skia/Point.cpp`) and `BBoxHierarchy.search` (located in the file `src/skia/Picture.cpp`), but the corresponding files do not include `#include `. Add the include statement and supplement with related tests. --- src/skia/Picture.cpp | 1 + src/skia/Point.cpp | 1 + tests/test_picture.py | 26 ++++++++++++++++++++++++-- tests/test_point.py | 7 +++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/skia/Picture.cpp b/src/skia/Picture.cpp index cdfe43a70..953587cf8 100644 --- a/src/skia/Picture.cpp +++ b/src/skia/Picture.cpp @@ -3,6 +3,7 @@ #include #include #include +#include namespace { diff --git a/src/skia/Point.cpp b/src/skia/Point.cpp index 9048f54fa..11b313203 100644 --- a/src/skia/Point.cpp +++ b/src/skia/Point.cpp @@ -1,6 +1,7 @@ #include "common.h" #include #include +#include void initPoint(py::module &m) { // IPoint diff --git a/tests/test_picture.py b/tests/test_picture.py index d1bc88340..91e68d74a 100644 --- a/tests/test_picture.py +++ b/tests/test_picture.py @@ -62,8 +62,30 @@ def test_Picture_MakePlaceholder(): skia.Picture.MakePlaceholder(skia.Rect(100, 100)), skia.Picture) -def test_PictureRecorder_init(): - assert isinstance(skia.PictureRecorder(), skia.PictureRecorder) +def test_RTreeFactory_init(): + assert isinstance(skia.RTreeFactory(), skia.RTreeFactory) + + +def test_RTreeFactory_call(): + rtree = skia.RTreeFactory()() + assert isinstance(rtree, skia.BBoxHierarchy) + + +@pytest.fixture +def rtree(): + return skia.RTreeFactory()() + + +def test_BBoxHierarchy_init(): + assert isinstance(skia.BBoxHierarchy(), skia.BBoxHierarchy) + + +def test_BBoxHierarchy_insert(rtree): + rtree.insert(skia.Rect(100, 100), 1) + + +def test_BBoxHierarchy_search(rtree): + rtree.search(skia.Rect(100, 100), []) @pytest.mark.parametrize('args', [ diff --git a/tests/test_point.py b/tests/test_point.py index e1f010eb0..6efcab49b 100644 --- a/tests/test_point.py +++ b/tests/test_point.py @@ -105,6 +105,13 @@ def test_Point_init(args): assert isinstance(skia.Point(*args), skia.Point) +def test_Point_Offset(): + points = [skia.Point(1, 2), skia.Point(3, 4)] + points = skia.Point.Offset(points, 1, 1) + assert points[0].equals(2, 3) + assert points[1].equals(4, 5) + + def test_Point_x(point): assert point.x() == 4 From ffcb64e5ba33f2aaf64d4de1ab9801398ce188bb Mon Sep 17 00:00:00 2001 From: meetwq Date: Tue, 17 Sep 2024 21:24:39 +0800 Subject: [PATCH 2/5] adjust signatures of `insert` and `search` methods in `BBoxHierarchy` pybind11 cannot fill an input pointer with fetched stuff. See #263 --- src/skia/Picture.cpp | 48 +++++++++++++++++++++++++++++++++++-------- tests/test_picture.py | 25 +++++++++++++--------- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/src/skia/Picture.cpp b/src/skia/Picture.cpp index 953587cf8..3111939dc 100644 --- a/src/skia/Picture.cpp +++ b/src/skia/Picture.cpp @@ -28,11 +28,32 @@ class PyPicture : public SkPicture { class PyBBoxHierarchy : public SkBBoxHierarchy { public: using SkBBoxHierarchy::SkBBoxHierarchy; + + // https://pybind11.readthedocs.io/en/stable/advanced/classes.html#different-method-signatures void insert(const SkRect rects[], int N) override { - PYBIND11_OVERRIDE_PURE(void, SkBBoxHierarchy, insert, rects, N); + pybind11::gil_scoped_acquire gil; + pybind11::function override = pybind11::get_override(this, "insert"); + if (override) { + override(std::vector(rects, rects + N)); + } + } + void insert(const SkRect rects[], const Metadata metadata[], int N) override { + pybind11::gil_scoped_acquire gil; + pybind11::function override = pybind11::get_override(this, "insert"); + if (override) { + override(std::vector(rects, rects + N), + std::vector(metadata, metadata + N)); + } } void search(const SkRect& query, std::vector *results) const override { - PYBIND11_OVERRIDE_PURE(void, SkBBoxHierarchy, search, query, results); + pybind11::gil_scoped_acquire gil; + pybind11::function override = pybind11::get_override(this, "search"); + if (override) { + auto obj = override(query); + if (py::isinstance(obj)) { + *results = obj.cast>(); + } + } } size_t bytesUsed() const override { PYBIND11_OVERRIDE_PURE(size_t, SkBBoxHierarchy, bytesUsed); @@ -290,21 +311,30 @@ py::class_(bboxhierarchy, "Metadata") bboxhierarchy .def(py::init()) .def("insert", - py::overload_cast(&SkBBoxHierarchy::insert), + [] (SkBBoxHierarchy& bbh, const std::vector& rects) { + return bbh.insert(rects.data(), rects.size()); + }, R"docstring( Insert N bounding boxes into the hierarchy. )docstring", - py::arg("rects"), py::arg("N")) + py::arg("rects")) .def("insert", - py::overload_cast(&SkBBoxHierarchy::insert), - py::arg("rects"), py::arg("metadata"), py::arg("N")) - .def("search", &SkBBoxHierarchy::search, + [] (SkBBoxHierarchy& bbh, const std::vector& rects, + const std::vector& metadata) { + return bbh.insert(rects.data(), metadata.data(), rects.size()); + }, + py::arg("rects"), py::arg("metadata")) + .def("search", + [] (SkBBoxHierarchy& bbh, const SkRect& query) { + std::vector results; + bbh.search(query, &results); + return results; + }, R"docstring( Populate results with the indices of bounding boxes intersecting that query. )docstring", - py::arg("query"), py::arg("results")) + py::arg("query")) .def("bytesUsed", &SkBBoxHierarchy::bytesUsed, R"docstring( Return approximate size in memory of this. diff --git a/tests/test_picture.py b/tests/test_picture.py index 91e68d74a..843a90cee 100644 --- a/tests/test_picture.py +++ b/tests/test_picture.py @@ -66,31 +66,36 @@ def test_RTreeFactory_init(): assert isinstance(skia.RTreeFactory(), skia.RTreeFactory) -def test_RTreeFactory_call(): - rtree = skia.RTreeFactory()() - assert isinstance(rtree, skia.BBoxHierarchy) +@pytest.fixture +def factory(): + return skia.RTreeFactory() -@pytest.fixture -def rtree(): - return skia.RTreeFactory()() +def test_RTreeFactory_call(factory): + bbh = factory() + assert isinstance(bbh, skia.BBoxHierarchy) def test_BBoxHierarchy_init(): assert isinstance(skia.BBoxHierarchy(), skia.BBoxHierarchy) -def test_BBoxHierarchy_insert(rtree): - rtree.insert(skia.Rect(100, 100), 1) +def test_BBoxHierarchy_insert(factory): + bbh = factory() + bbh.insert([skia.Rect(100, 100)]) -def test_BBoxHierarchy_search(rtree): - rtree.search(skia.Rect(100, 100), []) +def test_BBoxHierarchy_search(factory): + bbh = factory() + bbh.insert([skia.Rect(100, 100)]) + results = bbh.search(skia.Rect(100, 100)) + assert results[0] == 0 @pytest.mark.parametrize('args', [ (skia.Rect(100, 100),), (100, 100), + (skia.Rect(100, 100), skia.RTreeFactory()()), ]) def test_PictureRecorder_beginRecording(recorder, args): canvas = recorder.beginRecording(*args) From 9e814030f9aa43a0a47f57456d8307d02a02b86c Mon Sep 17 00:00:00 2001 From: meetwq Date: Wed, 18 Sep 2024 20:35:53 +0800 Subject: [PATCH 3/5] add `test_inherit_BBoxHierarchy` --- tests/test_picture.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/test_picture.py b/tests/test_picture.py index 843a90cee..9c3e1f2bb 100644 --- a/tests/test_picture.py +++ b/tests/test_picture.py @@ -92,6 +92,28 @@ def test_BBoxHierarchy_search(factory): assert results[0] == 0 +def test_inherit_BBoxHierarchy(): + class TestBBH(skia.BBoxHierarchy): + def __init__(self): + self.rects = [] + super().__init__() + + def search(self, query): + return [i for i, rect in enumerate(self.rects) if rect.intersects(query)] + + def insert(self, rects, metadata=[]): + self.rects.extend(rects) + + def bytesUsed(self): + return 0 + + bbh = TestBBH() + bbh.insert([skia.Rect(100, 100)]) + assert len(bbh.rects) == 1 + results = bbh.search(skia.Rect(100, 100)) + assert results[0] == 0 + + @pytest.mark.parametrize('args', [ (skia.Rect(100, 100),), (100, 100), From 02835da5042446da0f8881f83a694df3595e9c84 Mon Sep 17 00:00:00 2001 From: meetwq Date: Tue, 24 Sep 2024 09:50:12 +0800 Subject: [PATCH 4/5] add missing `test_PictureRecorder_init` It was deleted by mistake --- tests/test_picture.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_picture.py b/tests/test_picture.py index 9c3e1f2bb..e7cc55f48 100644 --- a/tests/test_picture.py +++ b/tests/test_picture.py @@ -114,6 +114,10 @@ def bytesUsed(self): assert results[0] == 0 +def test_PictureRecorder_init(): + assert isinstance(skia.PictureRecorder(), skia.PictureRecorder) + + @pytest.mark.parametrize('args', [ (skia.Rect(100, 100),), (100, 100), From 7e9947f55444c038ef569ac61e30cd6afe2d628b Mon Sep 17 00:00:00 2001 From: meetwq Date: Tue, 24 Sep 2024 10:12:38 +0800 Subject: [PATCH 5/5] additional tests for `Picture` port 3 tests `Picture_fillsBBH`, `PictureNegativeSpace` and `Picture_SkipBBH` from google/skia --- tests/test_picture.py | 79 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/test_picture.py b/tests/test_picture.py index e7cc55f48..b99cb422e 100644 --- a/tests/test_picture.py +++ b/tests/test_picture.py @@ -179,3 +179,82 @@ def test_Drawable_getBounds(drawable): def test_Drawable_notifyDrawingChanged(drawable): drawable.notifyDrawingChanged() + + +# https://github.com/google/skia/blob/ad08229fd0163a784c60a8bac2c0c5a6a13877c9/tests/PictureTest.cpp#L869-L893 +def test_Picture_fillsBBH(factory, recorder): + rects = [ + skia.Rect(0, 0, 20, 20), + skia.Rect(20, 20, 40, 40), + ] + + for n in range(len(rects) + 1): + bbh = factory() + + canvas = recorder.beginRecording(skia.Rect(0, 0, 100, 100), bbh) + for i in range(n): + canvas.drawRect(rects[i], skia.Paint()) + recorder.finishRecordingAsPicture() + + results = bbh.search(skia.Rect(0, 0, 100, 100)) + assert len(results) == n + + +# https://github.com/google/skia/blob/64148dd7cfe0a3f104d93f58ec42592a0252d378/tests/PictureBBHTest.cpp#L99-L127 +def test_PictureNegativeSpace(factory, recorder): + cull = skia.Rect(-200, -200, 200, 200) + + bbh = factory() + canvas = recorder.beginRecording(cull, bbh) + canvas.save() + canvas.clipRect(cull) + canvas.drawRect(skia.Rect(-20, -20, -10, -10), skia.Paint()) + canvas.drawRect(skia.Rect(-20, -20, -10, -10), skia.Paint()) + canvas.restore() + picture = recorder.finishRecordingAsPicture() + assert picture.approximateOpCount() == 5 + assert picture.cullRect() == skia.Rect(-20, -20, -10, -10) + + bbh = factory() + canvas = recorder.beginRecording(cull, bbh) + canvas.clipRect(cull) + canvas.drawRect(skia.Rect(-20, -20, -10, -10), skia.Paint()) + canvas.drawRect(skia.Rect(-20, -20, -10, -10), skia.Paint()) + picture = recorder.finishRecordingAsPicture() + assert picture.approximateOpCount() == 3 + assert picture.cullRect() == skia.Rect(-20, -20, -10, -10) + + +# https://github.com/google/skia/blob/64148dd7cfe0a3f104d93f58ec42592a0252d378/tests/PictureTest.cpp#L613-L657 +def test_Picture_SkipBBH(recorder): + class CountingBBH(skia.BBoxHierarchy): + def __init__(self): + self.search_calls: int = 0 + super().__init__() + + def search(self, query): + self.search_calls += 1 + + def insert(self, rects, metadata=[]): + pass + + def bytesUsed(self): + return 0 + + bound = skia.Rect(320, 240) + + bbh = CountingBBH() + + canvas = recorder.beginRecording(bound, bbh) + canvas.drawRect(bound, skia.Paint()) + canvas.drawRect(bound, skia.Paint()) + picture = recorder.finishRecordingAsPicture() + + big = skia.Canvas(640, 480) + small = skia.Canvas(300, 200) + + picture.playback(big) + assert bbh.search_calls == 0 + + picture.playback(small) + assert bbh.search_calls == 1