Skip to content
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

Add API to take snapshot for object #2353

Merged
merged 14 commits into from
Jul 19, 2021
Merged

Add API to take snapshot for object #2353

merged 14 commits into from
Jul 19, 2021

Conversation

XuNeo
Copy link
Collaborator

@XuNeo XuNeo commented Jul 6, 2021

Add lv_obj_take_snapshot API to take snapshot for an object together with its children.

The API will generate a snapshot image for object which can be directly set as lv_img source.

The side affect:

Assume the object for snapshot is called OBJ.
A fake display is used to generate snapshot, the parent of OBJ is changed to fake display, and then back to original parent when done. While the time it's back, its order in parent's children array(parent->spec_attr->children) is changed.

Posible solution is add another lv_obj_set_parent that keep its current position and set it back when finished.
Appreciate if any other ideas.
Updated to method kisvegabor provided, directly set object->parent and set it back when done.

Checkpoints

Below code is used for test.

static lv_obj_t* img_snapshot;

static void img_callback(lv_event_t* e)
{
    lv_obj_t * obj = lv_event_get_user_data(e);
    if (img_snapshot){
        lv_img_dsc_t* snapshot = (void*)lv_img_get_src(img_snapshot);
        if(snapshot){
            lv_snapshot_free(snapshot);
        }
        snapshot = lv_snapshot_take(obj, LV_IMG_CF_TRUE_COLOR_ALPHA);
        lv_img_set_src(img_snapshot, snapshot);
    }
}

void lv_snapshot_demo(void)
{
    LV_IMG_DECLARE(img_clothes);
    /* Avoid bug. */
    lv_refr_now(lv_disp_get_default());

    lv_obj_set_style_bg_color(lv_scr_act(), lv_color_hex(0x008080), 0);
    lv_obj_t* obj = lv_obj_create(lv_scr_act());

    lv_obj_align(obj, LV_ALIGN_CENTER, 0, 0);
    lv_obj_set_size(obj, 180, 180);
    lv_obj_set_flex_flow(obj, LV_FLEX_FLOW_ROW_WRAP);
    lv_obj_set_flex_align(obj, LV_FLEX_ALIGN_SPACE_EVENLY, LV_FLEX_ALIGN_CENTER, LV_FLEX_ALIGN_CENTER);
    lv_obj_set_style_radius(obj, 50, 0);
    lv_obj_t* img;
    int i;
    for(i = 0; i < 4; i++) {
        img = lv_img_create(obj);
        lv_img_set_src(img, &img_clothes);
        lv_obj_set_style_bg_color(img, lv_color_black(), 0);
        lv_obj_set_style_bg_opa(img, LV_OPA_COVER, 0);
        lv_obj_set_style_transform_zoom(img, 400, LV_STATE_PRESSED);
        lv_obj_add_flag(img, LV_OBJ_FLAG_CLICKABLE);
        lv_obj_add_event_cb(img, img_callback, LV_EVENT_PRESSED, obj);
        lv_obj_add_event_cb(img, img_callback, LV_EVENT_RELEASED, obj);
    }

    img_snapshot = lv_img_create(lv_scr_act());
    lv_obj_set_style_bg_color(img_snapshot, lv_color_hex(0xff0000), 0);
    lv_obj_set_style_bg_opa(img_snapshot, LV_OPA_50, 0);
}

Signed-off-by: Xu Xingliang <xuxingliang@xiaomi.com>
@codecov

This comment has been minimized.

@kisvegabor
Copy link
Member

kisvegabor commented Jul 6, 2021

That's great!

There one thing I'm not sure about. If the object has rounded corners the background is taken to the screenshot on the corners, right? It's even worse if the background is a wallpaper image and the object is moved from its actual position.

I haven't tested it yet but I think it should work:

  • create a new display as you did:
  • set bg_opa = 0 on the screen (i.e. delete all styles)
  • fake the parent manually by obj->parent = fake_screen (no coordinate change and fast)
  • clear the display buffer to have alpha=0 on every pixel
  • invalidate the object with lv_obj_invalidate (from the set_px_cb's point of view there is no difference as coordinates are relative to the "draw area")
  • use the draw buffer as the image

What do you think?

PS: sorry for the noise from codecov (code coverage report). We will remove it.

src/misc/lv_snapshot.h Outdated Show resolved Hide resolved
@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 7, 2021

Hi kisvegabor,
I have updated the code, using method you provided.

  • fake the parent manually by obj->parent = fake_screen (no coordinate change and fast)

Yes, this works, tricky thing is that we need to manully alloc parent obj->spec_attr and set its children to object for snapshot.

  • invalidate the object with lv_obj_invalidate (from the set_px_cb's point of view there is no difference as coordinates are relative to the "draw area")

I understand set_px_cb's relationship with draw area. However, if i don't change object position to 0,0, then it's outside of display(no common area with object act_scr, thus cannot generate image.

The hack method could be manually change object act_scr to have same coordinates with object for snapshot, however, I'm not full clear of how the system works, so I stick to the method of changing object position to 0,0 then back.
Also the align may also need to handle properly using hack method.

I have also updated the example in case you want to investigate more.

PS: sorry for the noise from codecov (code coverage report). We will remove it.

No problem at all.

snapshot-compressed

@XuNeo XuNeo changed the title WIP. add lv_obj_take_snapshot API. Add API to take snapshot for object Jul 8, 2021
@kisvegabor
Copy link
Member

I've just tested it and worked very well. I've pushed a fix to keep avoid repositioning.

We need to find a good folder for lv_snapshot. I think it should be in the src/extra folder but it doesn't fit into any of the existing folders there. Maybe we need an src/extra/others folder. What do you think?

@embeddedt
Copy link
Member

I don't see anything wrong with it being a core API in src/misc; the ability to take snapshots seems like a fairly useful feature. We can probably make use of it in the unit tests as well.

@amirgon
Copy link
Contributor

amirgon commented Jul 9, 2021

Wouldn't you want some macro for enabling/disabling it for saving program size?

Also, a Micropython example could be useful.

@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 9, 2021

When choose the folder for lv_snapshot, better to take repeated functions set_px_cb_alpha1, set_px_cb_alphaxxx into consideration, since these functions are directly copied from lv_canvas.c. Better to make them common for both lv_canvas and lv_snapshot.

For program size, could we rely on compiler to optimize it out when not used?

@embeddedt
Copy link
Member

The compiler can't optimize out APIs for MicroPython, as it automatically generates references to all of them and therefore they are never dead code.

@kisvegabor
Copy link
Member

I'm ok with treating it as a core. What about making it part of the lv_disp?

I think we need an API function like:

lv_snapshot_take(obj, cf, image_buf);

to allow saving the snapshot to any kind of memory.

IMO the set_px functions can be moved to lv_img_buf.c and used by the canvas and snapshot.

@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 15, 2021

I have moved the set_px_cb functions to lv_hal_disp.c, since these are generic set_px_cb methods that display driver could use.
lv_img_buf provides the ability to set pixel in buffer, and should not care about display related things, I.E. no need to know what the parameter lv_disp_drv_t means.
What do you think?

@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 15, 2021

I think we need an API function like:

lv_snapshot_take(obj, cf, image_buf);

to allow saving the snapshot to any kind of memory.

I have added API lv_snapshot_take_to_buff for this case.
Alse API lv_snapshot_buff_size_needed to calculate the buffer size needed, and can be used to resize the buffer if object size changed.

@kisvegabor
Copy link
Member

I have moved the set_px_cb functions to lv_hal_disp.c, since these are generic set_px_cb methods that display driver could use.

True, I agree.

I have added API lv_snapshot_take_to_buff for this case.

Looks good! Only one thing: on other places we use buf instead of buff. Could you modify it?

After that only two things are missing:

  • move lv_snapshot to an other folder (misc usually contains features unrelated to LVGL itself)
  • few paragraphs of documentation

kisvegabor and others added 2 commits July 15, 2021 20:11
1. Update parameter buff to buf.
2. Add macro to disable lv_snapshot, enabled by default.
@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 16, 2021

Hi @kisvegabor ,
I have moved source to extra/others/snapshot, changed buff to buf, added macro LV_USE_SNAPSHOT .

Where should I put the document to? Should I include a paragraph to object.md?
I also want to add an example for it. Would folder lvgl/examples/others/snapshot be appropriate?

Thanks.

@kisvegabor
Copy link
Member

I have moved source to extra/others/snapshot, changed buff to buf, added macro LV_USE_SNAPSHOT .

Great! Please also

  • add LV_USE_SNAPSHOT to lv_conf_template.h
  • run cd ..; ./lv_conf_internal_gen.py to update lv_conf_internal.h

Where should I put the document to?

I've just created a md file for it.

Should I include a paragraph to object.md?

Yes, good idea.

I also want to add an example for it. Would folder lvgl/examples/others/snapshot be appropriate?

Yes, it looks good.

@XuNeo XuNeo force-pushed the lv-snapshot branch 3 times, most recently from a589672 to abc6e41 Compare July 18, 2021 14:54
1. Update doc snapshot.md
2. Add example lv_example_snapshot_1 to folder examples/others/snapshot
3. Update lv_conf_template.h and lv_conf_internal.h
4. Remove lv_snapshot.c from lv_misc.mk
5. Add others to index.md

Signed-off-by: Xu Xingliang <xuxingliang@xiaomi.com>
@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 18, 2021

Hi @kisvegabor ,
I have updated docs with examples. CI has passed and I reviewed the pdf doc generated and it looks like what I want.
Could you kindly give a further review? Thanks.

Regards,

Edit: typo fix

@amirgon
Copy link
Contributor

amirgon commented Jul 18, 2021

@XuNeo Would you also provide a Micropython example for this feature?

@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 19, 2021

Hi @amirgon, there is no snapshot bindings generated. Where should I get started?

@amirgon
Copy link
Contributor

amirgon commented Jul 19, 2021

Hi @amirgon, there is no snapshot bindings generated. Where should I get started?

Basically you need to clone lv_micropython and checkout your PR on LVGL submodule:

  • git clone https://github.com/lvgl/lv_micropython.git
  • git submodule update --init --recursive lib/lv_bindings
  • cd lib/lv_bindings/lvgl
  • git fetch origin pull/2353/head:snapshot
  • git checkout snapshot
  • git merge master <--- This is needed if your PR is not up to date with latest LVGL
  • cd ../../..

Then you need to build the unix port:

  • make -C ports/unix VARIANT=dev DEBUG=1 submodules
  • make -j $(nproc) -C mpy-cross
  • make -j $(nproc) -C ports/unix VARIANT=dev DEBUG=1

Then you can run it with: ports/unix/micropython-dev -i <Your Script>

@XuNeo
Copy link
Collaborator Author

XuNeo commented Jul 19, 2021

Hi @amirgon ,
Micropython example has been added, this needs pull request #2382 to work properly.

Regards,

Copy link
Member

@kisvegabor kisvegabor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, all good from my side!

@amirgon, can I merge it?

@amirgon
Copy link
Contributor

amirgon commented Jul 19, 2021

@amirgon, can I merge it?

Yes.

@kisvegabor
Copy link
Member

Great, merging.

Thank you very much for your contribution.

@kisvegabor kisvegabor merged commit c98c825 into lvgl:master Jul 19, 2021
@XuNeo XuNeo deleted the lv-snapshot branch July 19, 2021 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants