-
Notifications
You must be signed in to change notification settings - Fork 540
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
Png chunk reader codec bitmap #2149
base: main
Are you sure you want to change the base?
Conversation
add /.idea to git ignore (mono#2138)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
All we need are some API tweaks and then some tests! The main areas for tests are around the pixmap values depending on whether bitmaps have pixel data or not. Also we need tests for various permutations of the chunk reader with some png files with actual tags in it.
…nto png_chunk_reader_codec_bitmap
tests/Tests/SKCodecTest.cs
Outdated
public void CanReadPngChunks_iTXt() | ||
{ | ||
var path = Path.Combine(PathToImages, "png_chunks/good_itxt.png"); | ||
|
||
using var chunkReader = new TestPngChunkReader(); | ||
using var codec = SKCodec.Create(path, chunkReader, SKCodecSelectionPolicy.PreferStillImage, out var codecResult); | ||
using var bmp = SKBitmap.Decode(codec); | ||
|
||
var chunks = chunkReader.Chunks; | ||
|
||
Assert.NotEmpty(chunks); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is failing because chunks are not being read still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you try the following?
internal class Reader : SKPngChunkReader
{
public bool r = false;
protected override bool ReadChunk(string tag, IntPtr data, IntPtr length)
{
r = true;
return true;
}
}
public void CanInvokePngReadChunks(
{
using Reader chunkReader = new();
var path = Path.Combine(PathToImages, "png_chunks/good_itxt.png");
using var codec = SKCodec.Create(path, chunkReader, SKCodecSelectionPolicy.PreferStillImage, out var codecResult);
Assert.True(r.r, "failed to invoke chunk reader");
}
with SkiaSharp\externals\skia\BUILD.gn
optional("png_decode") {
enabled = skia_use_libpng_decode
public_defines = [ "SK_CODEC_DECODES_PNG", "PNG_READ_UNKNOWN_CHUNKS_SUPPORTED" ]
deps = [ "//third_party/libpng" ]
sources = [
"src/codec/SkIcoCodec.cpp",
"src/codec/SkPngCodec.cpp",
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already set the flag here #2149 (comment)
I can actually debug and see it reading the chunk but it seems to decide it is not worth it and skips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oof
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://groups.google.com/g/skia-discuss/c/zHVOjwoQ-jQ/m/aKQ-EIZ_AQAJ
it looks like we will need to test it with UNKNOWN chunks instead of KNOWN chunks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
coincidentally this [an example implementation here](https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/jni/NinePatchPeeker.cpp).
is exactly what i have ported
https://github.com/mgood7123/AndroidUI/blob/master/AndroidUI/Graphics/NinePatchPeeker.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but i currently dont have any NinePatch images to test with
however the NinePatch (api itself) tests all pass https://github.com/mgood7123/android_9_patch/blob/31931f9910f428207ee482a808cc09195a985c87/9patch_tests.cpp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can borrow some images from android in the GitHub.com/aosp_mirror
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mgood7123 I see chunking wasn't working in 2 ways, so I opened this discussion: https://groups.google.com/g/skia-discuss/c/zHVOjwoQ-jQ
PNG chunk images are from https://www.nayuki.io/page/png-file-chunk-inspector |
fix string for unable to create instance
Based on the google discussion:
I am not sure this adds value since there is no way to read those values in the PNG. |
i think they meant skia does not give you a way to get the standard chunks (nor does libpng unless you manually decode by yourself) as unknown chunks SHOULD be supported since android used them to store NinePatch data |
I added a test, so before we merge I think we should add a test proving this works. Are you able to find an image that causes the chunks to be read? I can't merge unless we know for sure this is working. |
ill see what i can find |
i tried it with var bm = BitmapFactory.decodeFile(Context, image_path_NPatch);
var chunks = bm.getNinePatchChunk();
if (NinePatch.isNinePatchChunk(chunks))
{
Log.d("NINEPATCH", "IS CHUNK YES");
//image.setImageDrawable(new NinePatchDrawable(bm, chunks, new Rect(), null));
} else
{
Log.d("NINEPATCH", "IS CHUNK NO");
} but i get additionally i added protected override bool ReadChunk(string tag, IntPtr data, IntPtr length)
{
// NPatch
System.Console.WriteLine("READ NINEPATCH CHUNK"); but i dont see it anywhere |
in android this works Bitmap bm = decodeResource(getResources(), R.drawable.circle); // same as DecodeFile
byte[] ninePatchChunk = bm.getNinePatchChunk();
if (NinePatch.isNinePatchChunk(ninePatchChunk)) {
image.setImageDrawable(new NinePatchDrawable(bm, ninePatchChunk, new Rect(), null)); |
and it can read it :) internal class _10_test_png_chunk_reader : Test
{
internal class Reader : SKPngChunkReader
{
public bool r = false;
protected override bool ReadChunk(string tag, IntPtr data, IntPtr length)
{
r = true;
Console.WriteLine("READ CHUNK");
return true;
}
}
public override void Run(TestGroup nullableInstance)
{
using Reader r = new();
using FileStream stream = new("C:\\Users\\AndroidUI\\Desktop\\AndroidUI\\AndroidUITest\\png_chunks\\good_itxt.png", FileMode.Open);
SKCodecResult result_;
using SKCodec c = SKCodec.Create(stream, out result_, r);
Tools.ExpectFalse(r.r, "known chunks are not meant to invoke chunk reader");
using Reader r2 = new();
using FileStream np = new("K:/Images/9PNG/circle.9.png", FileMode.Open);
SKCodecResult result2_;
using SKCodec c2 = SKCodec.Create(np, out result2_, r2);
Tools.ExpectTrue(r2.r, "unknown chunks are meant to invoke chunk reader");
}
} |
Can you add this as a test? |
this should work now :) |
it seems so basically... chunk_name = png_ptr->chunk_name;
if (chunk_name == png_IDAT)
{
// ...
}
if (chunk_name == png_IHDR)
{
// ...
}
else if (chunk_name == png_IEND)
{
// ...
}
#ifdef PNG_HANDLE_AS_UNKNOWN_SUPPORTED
else if ((keep = png_chunk_unknown_handling(png_ptr, chunk_name)) != 0)
{
PNG_PUSH_SAVE_BUFFER_IF_FULL
png_handle_unknown(png_ptr, info_ptr, png_ptr->push_length, keep);
if (chunk_name == png_PLTE)
png_ptr->mode |= PNG_HAVE_PLTE;
}
#endif
else if (chunk_name == png_PLTE)
{
// ...
}
else if (chunk_name == png_IDAT)
{
// ...
return;
}
#ifdef PNG_READ_gAMA_SUPPORTED
else if (png_ptr->chunk_name == png_gAMA)
{
// ...
}
#endif
#ifdef PNG_READ_sBIT_SUPPORTED
else if (png_ptr->chunk_name == png_sBIT)
{
// ...
}
#endif
#ifdef PNG_READ_cHRM_SUPPORTED
else if (png_ptr->chunk_name == png_cHRM)
{
// ...
}
#endif
#ifdef PNG_READ_sRGB_SUPPORTED
else if (chunk_name == png_sRGB)
{
// ...
}
#endif
#ifdef PNG_READ_iCCP_SUPPORTED
else if (png_ptr->chunk_name == png_iCCP)
{
// ...
}
#endif
#ifdef PNG_READ_sPLT_SUPPORTED
else if (chunk_name == png_sPLT)
{
// ...
}
#endif
#ifdef PNG_READ_tRNS_SUPPORTED
else if (chunk_name == png_tRNS)
{
// ...
}
#endif
#ifdef PNG_READ_bKGD_SUPPORTED
else if (chunk_name == png_bKGD)
{
// ...
}
#endif
#ifdef PNG_READ_hIST_SUPPORTED
else if (chunk_name == png_hIST)
{
// ...
}
#endif
#ifdef PNG_READ_pHYs_SUPPORTED
else if (chunk_name == png_pHYs)
{
// ...
}
#endif
#ifdef PNG_READ_oFFs_SUPPORTED
else if (chunk_name == png_oFFs)
{
// ...
}
#endif
#ifdef PNG_READ_pCAL_SUPPORTED
else if (chunk_name == png_pCAL)
{
// ...
}
#endif
#ifdef PNG_READ_sCAL_SUPPORTED
else if (chunk_name == png_sCAL)
{
// ...
}
#endif
#ifdef PNG_READ_tIME_SUPPORTED
else if (chunk_name == png_tIME)
{
// ...
}
#endif
#ifdef PNG_READ_tEXt_SUPPORTED
else if (chunk_name == png_tEXt)
{
// ...
}
#endif
#ifdef PNG_READ_zTXt_SUPPORTED
else if (chunk_name == png_zTXt)
{
// ...
}
#endif
#ifdef PNG_READ_iTXt_SUPPORTED
else if (chunk_name == png_iTXt)
{
// ...
}
#endif
else
{
PNG_PUSH_SAVE_BUFFER_IF_FULL
png_handle_unknown(png_ptr, info_ptr, png_ptr->push_length,
PNG_HANDLE_CHUNK_AS_DEFAULT);
}
png_ptr->mode &= ~PNG_HAVE_CHUNK_HEADER; |
Description of Change
expose more api's
Bugs Fixed
None
API Changes
Added:
bool SKBitmap.SetInfo(SKImageInfo info)
bool SKBitmap.SetInfo(SKImageInfo info, int rowBytes)
bool SKBitmap.ComputeIsOpaque()
void SKBitmap.AllocPixels(SKImageInfo info)
void SKBitmap.AllocPixels(SKImageInfo info, int rowBytes)
void SKBitmap.AllocPixels(SKImageInfo info, SKBitmapAllocFlags flags)
SKImage SKBitmap.AsImage ()
uint SKBitmap.GenerationId()
SKPixmap SkBitmap.Pixmap { get; }
bool SKBitmap.ReadPixels (SKImageInfo info, IntPtr dstpixels, int rowBytes, int x, int y)
bool SKBitmap.ReadPixels(SKPixmap dstPixmap)
bool SKBitmap.ReadPixels(SKPixmap pixmap, int x, int y)
bool SKBitmap.WritePixels(SKPixmap pixmap)
bool SKBitmap.WritePixels(SKPixmap dstPixmap, int x, int y)
enum SKCodec.SelectionPolicy.preferStillImage
enum SKCodec.SelectionPolicy.preferAnimation
SKSizeI SKCodec.Dimensions { get; }
static SKCodec SKCodec.Create(string filename, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(string filename, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(string filename, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(string filename, out SKCodecResult result, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(string filename, out SKCodecResult result, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create (string filename, out SKCodecResult result, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(Stream stream, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(Stream stream, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(Stream stream, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(Stream stream, out SKCodecResult result, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(Stream stream, out SKCodecResult result, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(Stream stream, out SKCodecResult result, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(SKStream stream, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(SKStream stream, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(SKStream stream, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(SKStream stream, out SKCodecResult result)
static SKCodec SKCodec.Create(SKStream stream, out SKCodecResult result, SKPngChunkReader chunkReader)
static SKCodec SKCodec.Create(SKStream stream, out SKCodecResult result, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(SKStream stream, out SKCodecResult result, SKPngChunkReader chunkReader, SelectionPolicy selectionPolicy)
static SKCodec SKCodec.Create(SKData data, SKPngChunkReader chunkReader)
abstract bool SKPngChunkReader.ReadChunk(string tag, void* data, IntPtr length)
virtual bool SKPngChunkReader.ReadChunk (string tag, IntPtr data, IntPtr length)
virtual bool SKPngChunkReader.ReadChunk (void* tag, void* data, IntPtr length)
Behavioral Changes
None.
Required skia PR
mono/skia#88
PR Checklist