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

renderer/Scene: Scene paints modifications #2957

Open
lpogic opened this issue Nov 13, 2024 · 5 comments
Open

renderer/Scene: Scene paints modifications #2957

lpogic opened this issue Nov 13, 2024 · 5 comments
Assignees
Labels
APIs Update / Revise APIs feature New feature additions
Milestone

Comments

@lpogic
Copy link
Collaborator

lpogic commented Nov 13, 2024

For now, the only ways to modify scene paints are Result Scene::push(Paint* paint) and Result Scene::clear(bool free) methods. It is still possible to get scene paints via list<Paint*>& Scene::paints() and modify the list directly, but this is probably not the best way to do it because it breaks the reference counting.

My suggestion is to add some methods for more control over the paints in the scene, for example:

  • Result Scene::remove(Paint* paint) - removes paint from scene
  • Result Scene::insert(size_t index, Paint* paint) - inserts paint into scene at index
@lpogic lpogic added feature New feature additions APIs Update / Revise APIs labels Nov 13, 2024
@lpogic lpogic added this to ThorVG Nov 13, 2024
@hermet hermet added this to the 1.0 milestone Nov 14, 2024
@hermet
Copy link
Member

hermet commented Nov 20, 2024

See also: #2598

@hermet hermet mentioned this issue Nov 20, 2024
80 tasks
@hermet
Copy link
Member

hermet commented Nov 20, 2024

@lpogic Hello, this is my suggestion. Please let me know if this doesn't compatible with you. Thanks.

Addition

Result Scene::remove(Paint* paint);  //Remove the paint from the scene
Paint* Scene::remove(size_t idx);       //Remove the paint associated with the idx
size_t Scene::count();  //Return the current paint count
const Paint* Scene::get(size_t idx) const;   //Temporary access to the paint associated with the idx.
size_t Scene::insert(Paint* paint, size_t idx = SIZE_MAX);  //Insert the paint at a specific position, moving the paints behind it accordingly. Using SIZE_MAX will append the paint to the end of the paint list. API also returns the paint index.

Remove

Result Scene::push(Paint* paint);
std::list<Paint*>& Scene::paints();
Result Scene::clear(bool free = true);

@hermet hermet self-assigned this Nov 20, 2024
@hermet hermet moved this to In Progress in ThorVG Nov 20, 2024
@lpogic
Copy link
Collaborator Author

lpogic commented Nov 20, 2024

@hermet Such an API would be enough for me. Only thing is will there be control over paint free during remove? I manage memory externally and it's important to me that there is a way to disable automatic free.

@hermet
Copy link
Member

hermet commented Nov 20, 2024

@lpogic Please check Paint features:

    /**
     * @brief Increment the reference count for the Paint instance.
     *
     * This method increases the reference count of the Paint object, allowing shared ownership and control over its lifetime.
     *
     * @return The updated reference count after the increment by 1.
     *
     * @warning Please ensure that each call to ref() is paired with a corresponding call to unref() to prevent a dangling instance.
     *
     * @see Paint::unref()
     * @see Paint::refCnt()
     *
     * @since 1.0
     */
    uint8_t ref() noexcept;

    /**
     * @brief Decrement the reference count for the Paint instance.
     *
     * This method decreases the reference count of the Paint object by 1.
     * If the reference count reaches zero and the @p free flag is set to true, the Paint instance is automatically deleted.
     *
     * @param[in] free Flag indicating whether to delete the Paint instance when the reference count reaches zero.
     *
     * @return The updated reference count after the decrement.
     *
     * @see Paint::ref()
     * @see Paint::refCnt()
     *
     * @since 1.0
     */
    uint8_t unref(bool free = true) noexcept;

    /**
     * @brief Retrieve the current reference count of the Paint instance.
     *
     * This method provides the current reference count, allowing the user to check the shared ownership state of the Paint object.
     *
     * @return The current reference count of the Paint instance.
     *
     * @see Paint::ref()
     * @see Paint::unref()
     *
     * @since 1.0
     */
    uint8_t refCnt() const noexcept;

@lpogic
Copy link
Collaborator Author

lpogic commented Nov 20, 2024

@hermet In fact, this way I can ensure that the reference counter does not reset to zero until I decrement it externally. Thanks for the tip!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APIs Update / Revise APIs feature New feature additions
Projects
Status: In Progress
Development

No branches or pull requests

2 participants