search

kyamagu / skia-python

Python binding to Skia Graphics Library
https://kyamagu.github.io/skia-python/
BSD 3-Clause "New" or "Revised" License
245 stars 43 forks source link

Improve SamplingOptions, and adding test cases #240

Open HinTak opened 7 months ago

HinTak commented 7 months ago

Cc @pavpanchekha

Pull #236 now includes the complete SamplingOptions code from pull #181, plus a little extra/update. The last of the emulation of the previous FilterQuality is:

  The last of SkFilterQuality emulation is:
  FilterQuality.kHigh_SkFilterQuality   -> SamplingOptions(CubicResampler.Mitchell())
  FilterQuality.kMedium_SkFilterQuality -> 
SamplingOptions(FilterMode.kLinear, MipmapMode.kNearest)    // cpu
                                           or SamplingOptions(FilterMode.kLinear, MipmapMode.kLinear)  // gpu
  FilterQuality.kLow_SkFilterQuality    -> SamplingOptions(FilterMode.kLinear, SkMipmapMode.kNone)
  FilterQuality.kNone_SkFilterQuality   -> SamplingOptions(SkFilterMode.kNearest, SkMipmapMode.kNone)

So it turns out that the 4 filterquality settings is equivalent to 5 of the 2×3x2 =12 SamplingOptions combo (one extra, because one of them depending on whether it is rendering through CPU or GPU), and we might as well add the whole thing.

Please test the build artefact when it finishes building.

Still missing are some test code, and possibly docstring documentation too.

HinTak commented 7 months ago

@pavpanchekha the above snippet (duplicated from inline comments) will go into the m124 release note when it gets written. For now, this snipplet is the only "migration" guide. If you could contribute some test code (to go into the pytest "tests" directory), preferably using images from skia's resources/images directory (most of our current tests are written that way, using test files from skia/resources/), that would be useful.

HinTak commented 6 months ago

236 now contains tests for the 5 combos which corresponds to the old m87 FilterQuality settings. More to do, but these are sufficient to satisfy migration needs from m87. #236 can go out, while this stays open.

pavpanchekha commented 6 months ago

Just to clarify—the old FilterQuality options will no longer be supported, and users should migrate to SamplingOptions?

pavpanchekha commented 6 months ago

What kind of tests would be useful? Actually checking that images have been resized with various quality levels? Or just checking that drawing an image with a given sampling quality is possible?

HinTak commented 6 months ago

Yes: https://github.com/HinTak/skia-python/blob/m124-public/relnotes/README.m124.md

For now I have only added those which had m87 equivalent, and check that the 6 are valid: https://github.com/HinTak/skia-python/blob/m124-public/tests/test_samplingoptions.py

Some actually non-trivial tests closer to how users might use them would be nice.

pavpanchekha commented 6 months ago

Just a nit—in your migration instructions above you wrote SkFilterOptions and SkMipmapMode a few times, it's not supposed to have the Sk prefix.

HinTak commented 6 months ago

Yes, I realised the extra sk afterwards - https://github.com/kyamagu/skia-python/pull/236/commits/7f6e2bb9d23caaa43304f091cea56e97cd3ff51c - the readme is copied from the tests/tests_samplingoptions.py and should work literally (after adding some "skia." prefix, or doing from skia import * [Not recommended, as there might be collisions/shadowing]).