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

[draft]extension/window: Introduce tvg::Window based on GLFW #1690

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JSUYA
Copy link
Member

@JSUYA JSUYA commented Oct 6, 2023

Add an extension that uses GLFW-based tvg::Window.
This window has a canvas built into it.
The update() function receives a function that has tvg::Canvas as a parameter.
Users can build paint into the scene.
And call tvg::Window::loop().

Please refer to example/Window.cpp.

[APIs]
tvg::Window::gen(int width, int height, std::string name, tvg::CanvasEngine engine)
void close();
void resize(int width, int height);
void init(std::function<bool(tvg::Canvas*)> on_update);
void update(std::function<bool(tvg::Canvas*)> on_update);
static bool loop();

This idea was inspired by these:
https://github.com/DeriveSDK/experiments/tree/main/glfw_imgui_tvg_window
#1244 (comment)

@JSUYA
Copy link
Member Author

JSUYA commented Oct 6, 2023

tvg::Window usage

if (tvg::Initializer::init(tvgEngine, threads) == tvg::Result::Success) {
        window = tvg::Window::gen(WIDTH, HEIGHT, "GLFW Window Example");
        window->update([=](tvg::Scene* main_scene) {
            auto shape1 = tvg::Shape::gen();
            //...
            main_scene->push(move(shape1));
            auto picture = tvg::Picture::gen();
            //...
            main_scene->push(move(picture));
            return true;
        });
        window->run();
        window->close();
        tvg::Initializer::term(tvgEngine);
}

Screenshot from 2023-10-06 22-24-08

The code is not clean and tested yet. If you have any ideas, please let me know.

@JSUYA JSUYA marked this pull request as draft October 6, 2023 14:14
@hermet hermet added the feature New feature additions label Oct 8, 2023
@hermet
Copy link
Member

hermet commented Oct 9, 2023

Thanks. one feedback:

I assume multiple canvases(windows) with different engines.

So this api is more compromised?
-> tvg::Window::gen(w, h, "title", engine = CanvasEngine::Sw); ?

Copy link
Member

Choose a reason for hiding this comment

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

  1. glTexCoord2f(0, 0); glVertex2f(0, 0);
    OpenGL 1.1? I think that we should use GL2.0 at least

  2. static tvg::Window* instance_window;
    Single window instance? We should support multiple windows with different render engines

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. glTexCoord2f(0, 0); glVertex2f(0, 0);
    OpenGL 1.1? I think that we should use GL2.0 at least

Thank you for feedback :) . I will update it.

  1. static tvg::Window* instance_window;
    Single window instance? We should support multiple windows with different render engines

Actually, I didn't consider multi window.
I think we need more feature like a window manager.
How about like this?

auto window_manager = tvg::WindowManager::gen();

auto window1 = tvg::Window::gen(w, h, engine,"1", engine);
window1.update( ...);
window_manager->push(std::move(window1));

auto window2 = tvg::Window::gen(w, h, engine,"1", engine);
window2.update( ...);
window_manager->push(std::move(window2));

window_manager->run();
window_manager->close();

or
it can be changed to tvg:SingleWindow.

@SergeyLebedkin @hermet What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

@JSUYA I don't think window mgr concept here is necceesary...

I think this looks enough.

auto window1 = tvg::Window::gen(w, h, engine,"1", engine);
window1.run();

auto window2 = tvg::Window::gen(w, h, engine,"2", engine);
window2.run();

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we need to manage the loop for each window, I modified it to call the static method tvg::Window::loop().

tvg::Initializer::init(tvgEngine, threads);

window = tvg::Window::gen(WIDTH, HEIGHT, "GLFW Window Example 1 (Sw)", tvg::CanvasEngine::Sw);

window->init([](tvg::Canvas* canvas) {
    auto main_scene = tvg::Scene::gen();
    ...
    canvas->push(std::move(main_scene));
    return true;
});

window->update([](tvg::Canvas* canvas) {
    canvas->update();
    return true;
});

window2 = tvg::Window::gen(WIDTH, HEIGHT, "GLFW Window Example 2 (Gl)", tvg::CanvasEngine::Gl);

window2->init([](tvg::Canvas* canvas) {
    auto main_scene = tvg::Scene::gen();
    ...
    canvas->push(std::move(main_scene));
    return true;
});

window2->update([](tvg::Canvas* canvas) {
    canvas->update();
    return true;
});

tvg::Window::loop();

tvg::Initializer::term(tvgEngine);

image

+)
I haven't updated the GL API yet.
And calling multiple GL windows doesn't work yet.

(void*)buffer
);
glBegin(GL_QUADS);
glTexCoord2f(0, 0); glVertex2f(0, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the texture is meant to cover the hole window, I think it's more convenient to use glBlitFramebuffer

@capnm
Copy link
Collaborator

capnm commented Oct 10, 2023

Successfully tested on Debian 11 👍

Building the example depends on elementary.
Could this be e.g. restructuring the example folders avoided?

Currently the user have to copy and build it externally.

image

src/examples/Window.cpp Outdated Show resolved Hide resolved
src/examples/Window.cpp Outdated Show resolved Hide resolved
@JSUYA
Copy link
Member Author

JSUYA commented Oct 12, 2023

Thanks. one feedback:

I assume multiple canvases(windows) with different engines.

So this api is more compromised? -> tvg::Window::gen(w, h, "title", engine = CanvasEngine::Sw); ?

Hi Thanks feedback. I have questions :)

  • Does initializer have to be included in tvg::window::gen? or Should support multiple windows after init both SW and GL?

  • Do we allow this scenario? i didnt test yet.
    init(GL);
    init(SW);
    ...
    term(GL);
    term(SW);

@hermet
Copy link
Member

hermet commented Oct 12, 2023

Thanks. one feedback:
I assume multiple canvases(windows) with different engines.
So this api is more compromised? -> tvg::Window::gen(w, h, "title", engine = CanvasEngine::Sw); ?

Hi Thanks feedback. I have questions :)

  • Does initializer have to be included in tvg::window::gen? or Should support multiple windows after init both SW and GL?
  • Do we allow this scenario? i didnt test yet.
    init(GL);
    init(SW);
    ...
    term(GL);
    term(SW);
init (GL | SW);
window ....
term ();

Add an extension that uses GLFW-based tvg::Window.
This window has a canvas built into it.
The update() function receives a function that has tvg::Canvas as a parameter.
Users can build paint into the scene.
And call tvg::Window::loop().

Please refer to example/Window.cpp.

[APIs]
tvg::Window::gen(int width, int height, std::string name, tvg::CanvasEngine engine)
void close();
void resize(int width, int height);
void init(std::function<bool(tvg::Canvas*)> on_update);
void update(std::function<bool(tvg::Canvas*)> on_update);
static bool loop();

This idea was inspired by these:
https://github.com/DeriveSDK/experiments/tree/main/glfw_imgui_tvg_window
thorvg#1244 (comment)
@capnm
Copy link
Collaborator

capnm commented Jan 16, 2024

This branch has conflicts that must be resolved

Could you rebase to current tip?
Thanks!

@JSUYA JSUYA removed their assignment Apr 12, 2024
@JSUYA
Copy link
Member Author

JSUYA commented Apr 12, 2024

I know there are many members waiting for this task to progress, but I haven't been able to do it cleanly. I apologize for the delay.
If anyone wants to handle this, they can continue this PR or start anew.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature additions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants