-
Notifications
You must be signed in to change notification settings - Fork 49
Support SkFontArguments::Palette #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,9 @@ sk_sp<SkFontMgr> SkFontMgr_RefDefault() { | |
|
|
||
| using Axis = SkFontParameters::Variation::Axis; | ||
| using Coordinate = SkFontArguments::VariationPosition::Coordinate; | ||
| using Override = SkFontArguments::Palette::Override; | ||
| PYBIND11_MAKE_OPAQUE(std::vector<Coordinate>); | ||
| PYBIND11_MAKE_OPAQUE(std::vector<Override>); | ||
|
|
||
| namespace { | ||
|
|
||
|
|
@@ -65,6 +67,12 @@ void SetVariationPositionCoordinates( | |
| vp.coordinateCount = coords.size(); | ||
| } | ||
|
|
||
| void SetPaletteOverrides( | ||
| SkFontArguments::Palette& palette, | ||
| const std::vector<Override>& overrides) { | ||
| palette.overrides = overrides.empty() ? nullptr : &overrides[0]; | ||
| palette.overrideCount = overrides.size(); | ||
| } | ||
|
|
||
| py::tuple SkFontStyleSet_getStyle(SkFontStyleSet* self, int index) { | ||
| SkFontStyle style; | ||
|
|
@@ -342,6 +350,59 @@ variationposition | |
| &SkFontArguments::VariationPosition::coordinateCount) | ||
| ; | ||
|
|
||
| py::class_<SkFontArguments::Palette> palette( | ||
| fontarguments, "Palette", | ||
| R"docstring( | ||
| A a palette to use and overrides for palette entries. | ||
| )docstring"); | ||
|
|
||
| py::class_<Override>(palette, "Override") | ||
HinTak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .def(py::init( | ||
| [] (uint16_t index, SkColor color) { | ||
| return Override({index, color}); | ||
| }), | ||
| py::arg("index"), py::arg("color")) | ||
| .def("__repr__", | ||
| [] (const Override& self) { | ||
| return py::str("Override(index={}, color={})").format( | ||
| self.index, self.color); | ||
| }) | ||
| .def_readwrite("index", &Override::index) | ||
| .def_readwrite("color", &Override::color) | ||
| ; | ||
|
|
||
| py::bind_vector<std::vector<Override>>(palette, "Overrides"); | ||
HinTak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| palette | ||
| .def(py::init( | ||
| [] (int index, const std::vector<Override>& overrides) { | ||
| SkFontArguments::Palette palette; | ||
| palette.index = index; | ||
| SetPaletteOverrides(palette, overrides); | ||
| return palette; | ||
| }), | ||
| py::keep_alive<1, 3>(), | ||
| py::arg("index"), py::arg("overrides")) | ||
| .def(py::init( | ||
| [] (int index) { | ||
| return SkFontArguments::Palette({index, nullptr, 0}); | ||
| }), | ||
| py::arg("index")) | ||
| .def("__repr__", | ||
| [] (const SkFontArguments::Palette& self) { | ||
| return py::str("Palette(index={}, overrideCount={})").format( | ||
| self.index, self.overrideCount); | ||
| }) | ||
| .def_property("overrides", | ||
| [] (const SkFontArguments::Palette& self) { | ||
| return std::vector<Override>( | ||
| self.overrides, self.overrides + self.overrideCount); | ||
| }, | ||
| &SetPaletteOverrides) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't you need a keep_Alive policy here too? On the set method. |
||
| .def_readwrite("index", &SkFontArguments::Palette::index) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| .def_readonly("overrideCount", &SkFontArguments::Palette::overrideCount) | ||
| ; | ||
|
|
||
| fontarguments | ||
| .def(py::init<>()) | ||
| .def("setCollectionIndex", &SkFontArguments::setCollectionIndex, | ||
|
|
@@ -352,6 +413,19 @@ fontarguments | |
| actually be indexed collections of fonts. | ||
| )docstring", | ||
| py::arg("collectionIndex")) | ||
| .def("setPalette", &SkFontArguments::setPalette, | ||
| R"docstring( | ||
| Specify a palette to use and overrides for palette entries. | ||
| )docstring", | ||
| py::arg("palette")) | ||
| .def("setPalette", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also wonder whether this should be a read/write property instead. |
||
| [](SkFontArguments& self, int index) { | ||
| return self.setPalette(SkFontArguments::Palette({index, nullptr, 0})); | ||
| }, | ||
| R"docstring( | ||
| Specify a palette index to use. | ||
| )docstring", | ||
| py::arg("index")) | ||
| .def("setVariationDesignPosition", | ||
| &SkFontArguments::setVariationDesignPosition, | ||
| R"docstring( | ||
|
|
@@ -365,6 +439,7 @@ fontarguments | |
| )docstring", | ||
| py::arg("position")) | ||
| .def("getCollectionIndex", &SkFontArguments::getCollectionIndex) | ||
| .def("getPalette", &SkFontArguments::getPalette) | ||
| .def("getVariationDesignPosition", | ||
| &SkFontArguments::getVariationDesignPosition) | ||
| ; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,44 @@ def test_FontArguments_setCollectionIndex(fontarguments): | |
| fontarguments.setCollectionIndex(0) | ||
|
|
||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there needs to be a dozen more tests for each of the new properties/methods of the new classes added. |
||
| @pytest.mark.parametrize( | ||
| "index, overrides", | ||
| [ | ||
| (0, [(0, skia.ColorBLACK), (1, skia.ColorRED), (5, skia.ColorBLUE)]), | ||
| (1, []), | ||
| (2, None), | ||
| ], | ||
| ) | ||
| def test_FontArguments_setPalette(fontarguments, index, overrides): | ||
| if overrides is None: | ||
| palette = skia.FontArguments.Palette(index) | ||
| else: | ||
| palette = skia.FontArguments.Palette( | ||
| index, | ||
| skia.FontArguments.Palette.Overrides( | ||
| [ | ||
| skia.FontArguments.Palette.Override(*override) | ||
| for override in overrides | ||
| ] | ||
| ), | ||
| ) | ||
|
|
||
| len_overrides = len(overrides) if overrides is not None else 0 | ||
| assert palette.index == index | ||
| assert len(palette.overrides) == len_overrides | ||
| assert palette.overrideCount == len_overrides | ||
| assert repr(palette) == f"Palette(index={index}, overrideCount={len_overrides})" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably not have a repr test - since the |
||
| for i, override in enumerate(palette.overrides): | ||
| assert repr(override) == f"Override(index={override.index}, color={override.color})" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto on testing the precise form of |
||
| assert override.index == overrides[i][0] | ||
| assert override.color == overrides[i][1] | ||
| fontarguments.setPalette(palette) | ||
|
|
||
|
|
||
| def test_FontArguments_setPalette_Index(fontarguments): | ||
| fontarguments.setPalette(1) | ||
|
|
||
|
|
||
| def test_FontArguments_setVariationDesignPosition(fontarguments): | ||
| coordinates = skia.FontArguments.VariationPosition.Coordinates([ | ||
| skia.FontArguments.VariationPosition.Coordinate(0x00, 0.), | ||
|
|
@@ -58,6 +96,10 @@ def test_FontArguments_getCollectionIndex(fontarguments): | |
| assert isinstance(fontarguments.getCollectionIndex(), int) | ||
|
|
||
|
|
||
| def test_FontArguments_getPalette(fontarguments): | ||
| assert isinstance(fontarguments.getPalette(), skia.FontArguments.Palette) | ||
|
|
||
|
|
||
| def test_FontArguments_getVariationDesignPosition(fontarguments): | ||
| assert isinstance( | ||
| fontarguments.getVariationDesignPosition(), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.