agraef / pd-lua

Lua bindings for Pd, updated for Lua 5.3+
https://agraef.github.io/pd-lua/
GNU General Public License v2.0
47 stars 10 forks source link

pdlua graphics #35

Closed timothyschoen closed 6 months ago

timothyschoen commented 9 months ago

Hi Albert!

Today I started working on a basic graphics API for pdlua. The goal is a cross-flavour way to create GUI objects.

So far, this is going quite well: I'm able to do basic drawing in pd-vanilla and plugdata, using the same Lua code. It should be simple to port over to purr-data as well. It's also a much easier way to create GUI externals, no pd structs or tcl/tk knowledge required.

Screenshot 2023-12-20 at 03 28 56

This is very much WIP, not mergable yet. Not anymore!

What works

The API

Example for draggable circle:

local example = pd.Class:new():register("example")

function example:initialize(sel, atoms)
    self.inlets = 1
    self.circle_x = 10
    self.circle_y = 10
    self.gui = 1
    return true
end

function example:mouse_drag(x, y)
    self.circle_x = x - 15
    self.circle_y = y - 15
    self:repaint() 
end

function example:paint()
    gfx.set_color(255, 0, 0, 1)
    gfx.fill_all()

    gfx.set_color(0, 255, 0, 1)
    gfx.fill_ellipse(self.circle_x, self.circle_y, 30, 30)
end

function example:in_1_bang()
    self:repaint()
end

As you can see, you can define a paint() function, where you have to do your painting. Then, you can call repaint() whenever repainting needs to happen. ~If you call gfx.initialize() in the constructor, it will switch the object from text to GUI mode, but this can be done in a better way probably~ EDIT: it now works by settings the self.gui flag.

The supported functions right now are:

-- Callbacks you can define
paint()
mouse_down(x, y)
mouse_up(x, y)
mouse_move(x, y)
mouse_drag(x, y)

-- Functions you can call in gfx namespace
set_size(w, h)
set_color(r, g, b, a=1.0)

fill_ellipse(x, y, w, h)
stroke_ellipse(x, y, w, h)

fill_rect(x, y, w, h)
stroke_rect(x, y, w, h, line_width)

fill_rounded_rect(x, y, w, h, corner_radius)
stroke_rounded_rect(x, y, w, h, corner_radius, line_width)

draw_line(x1, y1 x2, y2, line_width)
draw_text(text, x, y, w, fontsize)

start_path(x, y)
line_to(x, y)
quad_to(x1, y1, x2, y2)
cubic_to(x1, y1, x2, y2, x3, y3)
close_path()
stroke_path(line_width)
fill_path()

fill_all()

translate(tx, ty)
scale(sx, sy)
reset_transform()
timothyschoen commented 9 months ago

https://github.com/agraef/pd-lua/assets/44585538/aafa62ea-cc8a-4126-8030-366d939d8f7b

timothyschoen commented 9 months ago

https://github.com/agraef/pd-lua/assets/44585538/8b056d25-b58b-4de2-a8e7-f26304b87248

The API and implementations for plugdata and pure-data are done now. I'm gonna add some documentation, and then it's ready for merge.

That is, if you want to merge this. If not, I'll move it to a branch or something, NP either way. This project has existed for over a decade, so I can understand it if this rocks the boat too much.

porres commented 8 months ago

@agraef did you get a chance to look at this? It is now in an official PlugData release and we'd like to have an update of pdlua in deken as well so Vanilla users can have fun with this. cheers

timothyschoen commented 7 months ago

Working on some API improvements:

The paint() function now passes in an argument that represents the graphics context:

function example:paint(g)
    local width, height = self:get_size()

    g.set_color(255, 255, 255, 1)
    g.fill_all()

    g.set_color(0, 0, 0, 1)
    g.fill_ellipse(10, 10, 8, 8(
end

This means that drawing calls can no longer be made from outside the paint() function, which was illegal to do anyway.

Also, paths have their own state now:

    local p = path.start(10, 20)
    p:line_to(40, 100)
    p:line_to(130, 90)
    p:close()

    g.set_color(180, 244, 4)
    g.fill_path(p)

This means you can now also store paths, and it makes managing multiple paths more simple.

"set_size" has been moved from the "gfx" namespace (which is only for internal use now) to the pd object class itself. I've also added a get_size() function.

function example:initialize(sel, atoms)
    self.inlets = 1
    self.outlets = 1
    self.gui = 1
    self:set_size(150, 150)
    local width, height = self:get_size()

    return true
end
agraef commented 7 months ago

Well, is this still WIP? Or is it finished now?

In any case, this looks rather intrusive, so I'd rather keep it in a separate branch for now.

agraef commented 7 months ago

I don't see any actual drawing code in there. Where does that happen? Any chance that this version still works with Purr Data?

timothyschoen commented 7 months ago

The drawing code is here: https://github.com/agraef/pd-lua/blob/e7d530b866a24a1d5d30303c8e0337ee7b2dbf7d/pdlua_gfx.h

It's currently a big #if to implement drawing for plugdata and pd-vanilla. It should be pretty trivial to add purr-data compatible drawing commands here too I think, but I couldn't find any API documentation for this (maybe I should have looked better).

There is still one thing I need to fix, which is that graphics translations work a bit differently in pd-vanilla and plugdata when you apply multiple different translations in a row. Other than that, I think this is done.

Here's some demos:

3D oscilloscope (by ben-wes)

https://github.com/agraef/pd-lua/assets/44585538/c1f6220b-6ba5-4000-bd55-95c8c19529f9

Replacement for else/circle:

https://private-user-images.githubusercontent.com/44585538/307366653-99c4c38e-648f-4c9e-a9f6-9a59d3bea553.mov?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDg5NTg3MzEsIm5iZiI6MTcwODk1ODQzMSwicGF0aCI6Ii80NDU4NTUzOC8zMDczNjY2NTMtOTljNGMzOGUtNjQ4Zi00YzllLWE5ZjYtOWE1OWQzYmVhNTUzLm1vdj9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDAyMjYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwMjI2VDE0NDAzMVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRjZmMwMWU2ZjBjNGRkZDNmMDliMTJiOTIwNDMwZDE2MzQ4MDAzOGMyYjQ5NTlhNjA4M2YxMzgwNDE2Y2RlY2UmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.CK8ztu5Z42f9Oak2B1_zDe_S2waAR6QL_Ebii0ryWBk

@porres and I would like to include this in ELSE either way, because this makes it a lot easier for us to write new graphics objects for ELSE:

We also had some other plans, like adding support for DSP and allowing you to write lua code directly into the [pdlua] object, both features inspired by [ofelia]. If this is too much change for an external this old, we can also create our own fork with a different name. Or we can merge it here, whatever you prefer.

timothyschoen commented 7 months ago

Stacking transforms being incorrect in pd-vanilla is fixed now too.

porres commented 7 months ago

yeah, this is in PlugData already and its staying and people are excited. I wouldn't need this for ELSE exactly. else/circle has an issue in PlugData and one way to deal with it would be to provide a coded object and I'd like that, I think it has advantages. But using this makes it easier and a quick short term solution.

Thing is, we also want and need this in Vanilla just so we can share stuff and of course I can have this in ELSE. Maybe a new object with a new name (just [lua]) and with all drastic changes that could even break. For one thing, I think ditching [pdluax] would be good, and I guess people would really like being able to do live coding in an object and having audio support for that and coded externals.

agraef commented 7 months ago

@timothyschoen wrote:

It's currently a big #if to implement drawing for plugdata and pd-vanilla. It should be pretty trivial to add purr-data compatible drawing commands here too I think, but I couldn't find any API documentation for this (maybe I should have looked better).

Ah ok, I see. I think that the way forward would be to add some #if PURR_DATA conditionals for Purr Data with dummy implementations of all the graphics routines so that this version of pd-lua at least builds and runs inside Purr Data, even if the graphics won't work at present. Could you still add that? Then I could merge this into a testing branch, and put it out as a preview release for people to test -- and Alexandre could upload it to Deken for the vanilla users.

And yes, I'm afraid that it's going to be quite a bit of work to get the graphics actually working in Purr Data, but we could maybe put this on the agenda for a future Purr Data GSoC project, for instance. Also, I might have a look at it myself if I find the time, or find a good student with the necessary JS foo who wants to do this as a BSc or Master thesis.

@porres wrote:

yeah, this is in PlugData already and its staying and people are excited

No worries, I fully agree that this is a great addition! :) I can understand why people are excited about it. Gem is a big unwieldy beast with some cross-platform issues, and Pd/Pd-L2ork data structs are not exactly easy either. So a cross-flavor and cross-platform way to program basic graphics and custom guis with pd-lua would be great feature indeed.

agraef commented 7 months ago

BTW, those demos look damn cool. :) Are the demo patches available somewhere? And is there a build of PlugData with the modified pd-lua that I can download somewhere? I'd really like to give this a whirl myself asap.

agraef commented 7 months ago

@timothyschoen I just pulled your branch and built it on my Manjaro system, but after opening pdlua-help.pd in vanilla Pd (Pd version 0.54.1 straight from the Arch repos) it immediately segfaults when trying to open the pd graphics subpatch (which presumably loads hello-gui.pd_lua in the pdlua subdir). Here's the gdb backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff78ba0b5 in realloc () from /usr/lib/libc.so.6
(gdb) bt
watchdog: signaling pd...

#0  0x00007ffff78ba0b5 in realloc () from /usr/lib/libc.so.6
#1  0x0000555555603539 in resizebytes (old=<optimized out>, oldsize=1, 
    newsize=12) at /usr/src/debug/pd/pure-data-0.54-1/src/m_memory.c:52
#2  0x00007ffff72fc5fa in start_path () from ./pdlua.pd_linux
#3  0x00007ffff7327d95 in luaD_precall () from ./pdlua.pd_linux
#4  0x00007ffff730c897 in luaV_execute () from ./pdlua.pd_linux
#5  0x00007ffff7329739 in f_call () from ./pdlua.pd_linux
#6  0x00007ffff7305ccc in luaD_rawrunprotected () from ./pdlua.pd_linux
#7  0x00007ffff733b94a in lua_pcallk () from ./pdlua.pd_linux
#8  0x00007ffff7302bae in pdlua_gfx_repaint () from ./pdlua.pd_linux
#9  0x00005555555b31c1 in canvas_map (x=0x5555557afce0, f=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/g_canvas.c:801
#10 0x000055555560a6c1 in pd_typedmess (x=0x5555557afce0, s=<optimized out>, 
    argc=<optimized out>, argv=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_class.c:1087
#11 0x000055555560b331 in binbuf_eval (x=<optimized out>, 
    target=0x55555579ab60, argc=<optimized out>, argv=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_binbuf.c:764
#12 0x000055555561b63f in socketreceiver_read (x=0x555555757420, fd=24)
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:698
#13 0x00005555556110b4 in sys_domicrosleep (microsec=microsec@entry=0)
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:241
#14 0x0000555555613073 in sys_pollgui ()
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:1051
--Type <RET> for more, q to quit, c to continue without paging--
#15 0x0000555555618496 in m_pollingscheduler ()
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_sched.c:348
#16 m_mainloop () at /usr/src/debug/pd/pure-data-0.54-1/src/m_sched.c:429
#17 0x00007ffff7843cd0 in ?? () from /usr/lib/libc.so.6
#18 0x00007ffff7843d8a in __libc_start_main () from /usr/lib/libc.so.6
#19 0x000055555556c0c5 in _start ()

Looks like a memory allocation issue to me. Can you reproduce this?

timothyschoen commented 7 months ago

@timothyschoen I just pulled your branch and built it on my Manjaro system, but after opening pdlua-help.pd in vanilla Pd (Pd version 0.54.1 straight from the Arch repos) it immediately segfaults when trying to open the pd graphics subpatch (which presumably loads hello-gui.pd_lua in the pdlua subdir). Here's the gdb backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff78ba0b5 in realloc () from /usr/lib/libc.so.6
(gdb) bt
watchdog: signaling pd...

#0  0x00007ffff78ba0b5 in realloc () from /usr/lib/libc.so.6
#1  0x0000555555603539 in resizebytes (old=<optimized out>, oldsize=1, 
    newsize=12) at /usr/src/debug/pd/pure-data-0.54-1/src/m_memory.c:52
#2  0x00007ffff72fc5fa in start_path () from ./pdlua.pd_linux
#3  0x00007ffff7327d95 in luaD_precall () from ./pdlua.pd_linux
#4  0x00007ffff730c897 in luaV_execute () from ./pdlua.pd_linux
#5  0x00007ffff7329739 in f_call () from ./pdlua.pd_linux
#6  0x00007ffff7305ccc in luaD_rawrunprotected () from ./pdlua.pd_linux
#7  0x00007ffff733b94a in lua_pcallk () from ./pdlua.pd_linux
#8  0x00007ffff7302bae in pdlua_gfx_repaint () from ./pdlua.pd_linux
#9  0x00005555555b31c1 in canvas_map (x=0x5555557afce0, f=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/g_canvas.c:801
#10 0x000055555560a6c1 in pd_typedmess (x=0x5555557afce0, s=<optimized out>, 
    argc=<optimized out>, argv=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_class.c:1087
#11 0x000055555560b331 in binbuf_eval (x=<optimized out>, 
    target=0x55555579ab60, argc=<optimized out>, argv=<optimized out>)
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_binbuf.c:764
#12 0x000055555561b63f in socketreceiver_read (x=0x555555757420, fd=24)
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:698
#13 0x00005555556110b4 in sys_domicrosleep (microsec=microsec@entry=0)
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:241
#14 0x0000555555613073 in sys_pollgui ()
    at /usr/src/debug/pd/pure-data-0.54-1/src/s_inter.c:1051
--Type <RET> for more, q to quit, c to continue without paging--
#15 0x0000555555618496 in m_pollingscheduler ()
    at /usr/src/debug/pd/pure-data-0.54-1/src/m_sched.c:348
#16 m_mainloop () at /usr/src/debug/pd/pure-data-0.54-1/src/m_sched.c:429
#17 0x00007ffff7843cd0 in ?? () from /usr/lib/libc.so.6
#18 0x00007ffff7843d8a in __libc_start_main () from /usr/lib/libc.so.6
#19 0x000055555556c0c5 in _start ()

Looks like a memory allocation issue to me. Can you reproduce this?

Seems plausible that there's a bug there, I recently modified the paths system. I haven't had this crash yet, but I bet it will come out if I turn on AddressSanitizer.

The 3d oscilloscope is by @ben-wes, I don't have it myself. He also made a .obj model loader with it, pretty insane!

Here's the circle slider

local circle = pd.Class:new():register("circle")

function circle:initialize(sel, atoms)
    self.slider_position_x = 75
    self.slider_position_y = 75
    self.inlets = 1
    self.outlets = 1
    self.gui = 1
    self:set_size(150, 150)
    return true
end

function circle:mouse_down(x, y)
    self.mouse_down_x = x;
    self.mouse_down_y = y;
    self.mouse_down_slider_x = self.slider_position_x;
    self.mouse_down_slider_y = self.slider_position_y;
end

function circle:mouse_drag(x, y)
    local width, height = self.get_size()

    local dx = x - self.mouse_down_x
    local dy = y - self.mouse_down_y

    local new_x = (self.mouse_down_slider_x + dx) - width / 2;
    local new_y = (self.mouse_down_slider_y + dy) - height / 2;

    local distance = math.sqrt(new_x^2 + new_y^2)
    local angle = math.atan(new_y, new_x)

    distance = math.max(0, math.min(width / 2, distance))

    self.slider_position_x = math.cos(angle) * distance + (width / 2)
    self.slider_position_y = math.sin(angle) * distance + (height / 2)

    self:outlet(1, "list", {self.slider_position_x / width, self.slider_position_y / height})
    self:repaint()
end

function circle:paint(g)
    local width, height = self:get_size()

    g.set_color(Colors.background)
    g.fill_all()

    g.set_color(Colors.foreground)
    g.fill_ellipse(self.slider_position_x - 4, self.slider_position_y - 4, 8, 8)

    g.stroke_ellipse(1, 1, width - 2, height - 2, 1)
    g.draw_line(width / 2, 0, width / 2, height, 1);
    g.draw_line(0, height / 2, width, height / 2, 1);

    g.draw_line(self.slider_position_x, self.slider_position_y, self.slider_position_x, height / 2, 2);
    g.draw_line(self.slider_position_x, self.slider_position_y, width / 2, self.slider_position_y, 2);
end
porres commented 7 months ago

And is there a build of PlugData with the modified pd-lua that I can download somewhere?

any nightly build (0.8.4) or even the latest stable release (0.8.3).

we could maybe put this on the agenda for a future Purr Data GSoC project, for instance.

if you don't mind me asking, what is the current situation of Purr Data development. It seems pretty quiet, nothing really happened in the last year (and no GSoC, huh?). I also realize most active developments did happen during summer, cause of GSoC, but I don't remember what happened in 2022. Also, the situation with Pd-L2ork is confusing, they have an independent development, and would a GSoC be able to attend both projects?

Anyway, I wonder if PlugData could also apply and get help for GSoC contributions, do you know anything about it @timothyschoen ?

timothyschoen commented 7 months ago

@agraef Turns out this is a platform specific difference. Calling realloc without allocating beforehand is allowed on macOS, but not on Linux.

Should be fixed with 2a62d4e

ben-wes commented 7 months ago

attaching a current version of the oscilloscope (no guarantees concerning lua code quality!):

3d_osci_pdlua.zip

i admit that i switched to Gem for my visuals for now. but these projects here are absolutely incredible (pdlua and @timothyschoen's graphical extensions)! thank you so much for this!

the .obj loader was actually patched in vanilla pd (based on [text]) and i don't get it to run properly anymore right now. :/ but it used the same "engine" and drew lines for .obj lines and faces.

timothyschoen commented 7 months ago

My last big commit (promise!)

This brings signal processing support, for example:

local lua_sig = pd.Class:new():register("lua_sig")

function lua_sig:initialize(sel, atoms)
    self.inlets = {SIGNAL, DATA, DATA}
    self.outlets = {SIGNAL}
    self.phase = 0
    return true
end

-- dsp method is called when processing is about to start
function lua_sig:dsp(samplerate, blocksize)
    self.samplerate = samplerate
end

-- perform method gets called with a table for each signal inlet
-- you must return a table for each signal outlet
function lua_sig:perform(in1)
    local frequency = 220
    local amplitude = 0.5

    local angular_freq = 2 * math.pi * frequency / self.samplerate

    -- Loop through each sample index
    for i = 1, #in1 do
        in1[i] = amplitude * math.sin(self.phase)
        self.phase = self.phase + angular_freq
        if self.phase >= 2 * math.pi then
            self.phase = self.phase - 2 * math.pi
        end
    end

    return in1
end

This is backwards compatible: if you use inlets = 1, it will just create a data inlet. The source and API for this are pretty much directly stolen from ofelia.

agraef commented 6 months ago

Turns out this is a platform specific difference. Calling realloc without allocating beforehand is allowed on macOS, but not on Linux.

Sure is (from man realloc):

If ptr is NULL, then the call is equivalent to malloc(size), for all values of size.

But you can clearly see that something fishy was going on in the gdb backtrace:

#1  0x0000555555603539 in resizebytes (old=<optimized out>, oldsize=1, 
    newsize=12) at /usr/src/debug/pd/pure-data-0.54-1/src/m_memory.c:52

Unfortunately, the old pointer value isn't shown in the backtrace, but the oldsize of 1 gives it away. So either there was some unitialized memory being passed there, or something is wrong with Pd's resizebytes() -- which I consider highly unlikely, given how old and well-tested this code is.

Anyway, your commit https://github.com/agraef/pd-lua/commit/2a62d4eb1e4a486f91d9959849e5d140a16d53c4 indeed seems to have fixed this bug, and the subpatch now opens correctly in vanilla Pd and appears to work alright (I still have to try the other examples).

However, the plugin still doesn't load in Purr Data, it shows the following error in the console (from libdir_loader, apparently):

error: ./pdlua.pd_linux: ./pdlua.pd_linux: undefined symbol: glist_getzoom

We still need to get this fixed before I can merge this. The solution would be, as I suggested earlier, to sprinkle the source with #if PURR_DATA and provide dummy implementations of all the graphic routines, so that pd-lua doesn't try to dl-link against vanilla Pd functions which don't exist in Purr Data. And also to avoid compile errors on Windows, when linking against Purr Data. Can you still do this, please?

Other than that, the PR looks fine to me, so I'm inclined to merge it, after I have given it some more thorough testing. Of course it would be good to include some more graphics examples in the distribution. And I'll need to update the tutorial to discuss the new features so that people know how to use them. Which should then be merged back into the version that you include with plugdata.

agraef commented 6 months ago

This brings signal processing support

Wow. This is nice! I have been thinking about this myself some time ago, but shied away from it. ;-) This would certainly be useful to Purr Data users as well, so we really need to get this version working with Purr Data even if only for that feature.

image

ben-wes commented 6 months ago

that was a sudden appearance of signal rate here - really cool! fyi ... i adapted this oscilloscope to this now and (maybe/hopefully) cleaned it up a bit.

could get variable size, sampling intervals and buffer size. but i'll leave it here like this for now:

image osci3d.pd_lua.zip

timothyschoen commented 6 months ago

that was a sudden appearance of signal rate here - really cool! fyi ... i adapted this oscilloscope to this now and (maybe/hopefully) cleaned it up a bit.

could get variable size, sampling intervals and buffer size. but i'll leave it here like this for now:

image osci3d.pd_lua.zip

Oh sweet, I had to apply a bit more data reduction to make it work inside plugdata, but really awesome!

ben-wes commented 6 months ago

here's one little funny thing happening when dynamically resizing the canvas in vanilla Pd: the cords are not redrawn in this case:

Screenshot 2024-02-27 at 23 02 41
porres commented 6 months ago

the cords are not redrawn in this case:

seems you need canvas_fixlinesfor

https://github.com/pure-data/pure-data/blob/0259d00373b9e6f3bc1d76eebc5b243283ca7063/src/g_canvas.h#L490

agraef commented 6 months ago

that was a sudden appearance of signal rate here - really cool!

Yep, that was a pleasant surprise. :)

@ben-wes, thanks a bunch for this nice example! Can you also post the beefed-up version with all the extra messages on the last inlet?

ben-wes commented 6 months ago

Can you also post the beefed-up version with all the extra messages on the last inlet?

sure! i put it here now: https://github.com/ben-wes/osci3d-

agraef commented 6 months ago

Awesome! We should include that in the pd-lua examples somewhere, if you don't mind. I should also link to your repo in the tutorial, so that people know where to get the latest version.

seems you need canvas_fixlinesfor

Yes, that should probably be exposed in the graphics interface. @timothyschoen?

ben-wes commented 6 months ago

Awesome! We should include that in the pd-lua examples somewhere, if you don't mind.

@agraef : sure, would be happy about it! i guess, i should add some checks for the messages then and limits to make it a bit less error prone. will do that ... but feel free to include it in any way you want.

timothyschoen commented 6 months ago

seems you need canvas_fixlinesfor

Yes, that should probably be exposed in the graphics interface. @timothyschoen?

My goal is to hide all the pd specific calls behind a more generic graphics interface. So instead of exposing fixlinesfor, I've added a call to canvas_fixlinesfor when the user calls set_size. That should fix it!

agraef commented 6 months ago

@timothyschoen That did it! Works perfectly now:

osci3d

But maybe you could squash those 3 commits into one? No need to riddle the history with trivial fixes for editing blunders. Just sayin. :)

@ben-wes: I think that the Lua code is perfect right now as an example, it's more readable that way. But that shouldn't keep you from improving it, of course.

timothyschoen commented 6 months ago

@timothyschoen That did it! Works perfectly now:

osci3d osci3d

But maybe you could squash those 3 commits into one? No need to riddle the history with trivial fixes for editing blunders. Just sayin. :)

@ben-wes: I think that the Lua code is perfect right now as an example, it's more readable that way. But that shouldn't keep you from improving it, of course.

Good call, done

agraef commented 6 months ago

Oops, I actually meant fixup (f), not squash (s), maybe you could just remove those extra lines in the commit message now.

timothyschoen commented 6 months ago

Minor change: you no longer need to set self.gui = 1, it now checks if the paint() function is defined instead. That makes sense to me because you could see it as overriding the default paint function (which would draw the text object).

But I think now I'm really done with everything

agraef commented 6 months ago

@timothyschoen I have a question concerning multi-instance support. If I understand this correctly, this is needed so that each Pd instance gets its own Lua interpreter instance? But you only get more than one instance with libpd, right?

agraef commented 6 months ago

I think that with the latest changes, you also still need to revise doc/graphics.txt, as self.gui isn't needed any more.

agraef commented 6 months ago

Ok, let me try to summarize what we actually got in this PR (I'll need this for the release announcement):

Is that correct? Did I miss anything important?

Note to self (things to be done once this is merged):

After that, do a new release, so that the final source can be pulled back into purr-data and plugdata, and that Alexandre can put out a new package on Deken, so that all flavors are back in sync.

So there's still some way to go, but I can hopefully work on this during spring before the next semester begins.

NB: I'd still like to squash the PR down a little, as its history is a long and winded road, but that's probably futile now given that the source was refactored and revised a few times. So it's probably better to keep the history as it is, so that we can at least trace back what was going on there in the future.

agraef commented 6 months ago

@timothyschoen, can you rebase on current master please? I just merged another PR, so this one has some trivial conflicts now.

timothyschoen commented 6 months ago

@timothyschoen, can you rebase on current master please? I just merged another PR, so this one has some trivial conflicts now.

There was only a single newline conflict, so I just merged the new changes it in. Sorry, I'm not the biggest git hero ;)

Ok, let me try to summarize what we actually got in this PR (I'll need this for the release announcement):

* graphics support, allowing you to draw basic vector graphics and receive mouse events on pure-data and plugdata (purr-data support pending)

* multi-instance support for libpd-based implementations (plugdata, in particular)

* signal inlet/outlet support

Is that correct? Did I miss anything important?

Note to self (things to be done once this is merged):

* Make the present implementation work with purr-data again (`#if PURR_DATA` conditionals in the code, dummy graphics implementation).

* Test all existing examples, to make sure that there are no regressions.

* Add some graphics examples.

* Add sections about graphics and signal iolets to the tutorial.

After that, do a new release, so that the final source can be pulled back into purr-data and plugdata, and that Alexandre can put out a new package on Deken, so that all flavors are back in sync.

So there's still some way to go, but I can hopefully work on this during spring before the next semester begins.

NB: I'd still like to squash the PR down a little, as its history is a long and winded road, but that's probably futile now given that the source was refactored and revised a few times. So it's probably better to keep the history as it is, so that we can at least trace back what was going on there in the future.

I think that covers it all. Just pushed a last fix for multi-instance support, that wasn't completely done yet (it had a 1/2048 chance of failing).

We could try squashing this, it does have a lot of commits, though I find messing with git history a bit scary. But I'm not sure how much of this history is really useful. The code for graphics isn't that hard to understand anyway.

timothyschoen commented 6 months ago

@timothyschoen I have a question concerning multi-instance support. If I understand this correctly, this is needed so that each Pd instance gets its own Lua interpreter instance? But you only get more than one instance with libpd, right?

We use the lua_newthread(lua_state) function to create a new Lua thread with its own execution stack, but that still shares all its global variables with the state that is passed in as the first argument. This happens to be exactly what we need, because the class initialisation only happens once, on the main lua state, and our new state still need to be able to access that. Then we use a simple associative container to link the current pd instance to a lua thread, and use the __L() function to return the lua thread for the currently active pd instance.

So we got lucky that lua has a function that does exactly what we needed!

agraef commented 6 months ago

Just pushed a last fix for multi-instance support, that wasn't completely done yet (it had a 1/2048 chance of failing).

That's oddly specific. ;-) But I see what you mean.

We could try squashing this, it does have a lot of commits, though I find messing with git history a bit scary. But I'm not sure how much of this history is really useful. The code for graphics isn't that hard to understand anyway.

I'd hesitate to just squash it all into just a single commit. Ideally, we should have one commit for each feature (graphics, multi-instance support, signal iolet support), but I'm not sure it's even possible to untangle the corresponding commits at this point. I can give it the old college try, though. :)

We use the lua_newthread(lua_state) function to create a new Lua thread with its own execution stack, but that still shares all its global variables with the state that is passed in as the first argument. This happens to be exactly what we need

I'm sure glad that you figured this out, since I would have been at a loss there. So this solves #9 then.

porres commented 6 months ago

After that, do a new release, so that the final source can be pulled back into purr-data and plugdata, and that Alexandre can put out a new package on Deken, so that all flavors are back in sync.

So there's still some way to go, but I can hopefully work on this during spring before the next semester begins.

Actually, since PlugData 0.8.3 came out with this feature, we've been meaning to put this ASAP into ELSE in a next update in sync with 0.8.4, and I got an ELSE update ready by now that also finally includes some eurorack inspired modules I've been working on for about a year and that PlugData has already been offering it, so it's kinda all urgent for us to keep things in sync and compatible between Vanilla/ELSE and PlugData. I'm kinda ready to go but Tim needs like a week still to put 0.8.4 out.

Mostly, the graphics part is something that will help us and we're already working on a new version of [else/circle] that works well for both vanilla and plugdata, and some new objects are planned, like ben's 3D oscillator and more, so we just can't really wait I guess, sorry.

So, ELSE already has a modified version that I think it makes better sense. The 'lua loader' is just installed and works when the 'else' lib is called in startup or via [declare], so it makes things quite easy. As it is, we're simplifying things and not really shipping a [pdlua] object, or even [pdluax] (which just won't be supported as a feature). The idea is to just have the new [lua] object for live scripting into the object box, which is also something we wanna put out in the wild for people to test and have fun.

I think it makes sense to have a modified version as a separate fork that is simplified and doesn't really need to carry full compatibility and share [pdlua] or [pdluax], but it is similar nonetheless and an alternative. We can play back and forth and share things/developments, where [pdlua] can eventually do the same things we have in PlugData/ELSE and be shipped separately in deken and also be there for PurrData and Pd-l2ork,

Does this make sense?

agraef commented 6 months ago

we've been meaning to put this ASAP into ELSE

@porres Oh boy, there we go again. We already had this discussion the last time when you shipped an out-of-date and broken version of pd-lua with ELSE and refused to update it. Only this time it's actually worse as you're removing features from pd-lua, so your fork won't even be compatible.

Of course this is open source, so you can do whatever you like as long as you don't violate the GPL. But you're effectively forcing people to decide whether they want to run ELSE with its bundled fork of pd-lua, or upstream pd-lua. That's a very bad thing, and goes completely against the spirit of open source.

If you want to fork pd-lua and maintain your own version, that's perfectly fine, as long as people still have the choice which version to use. But forcing your fork down people's throat by bundling it with ELSE? I consider this actively hostile, not just directed against me and other upstream developers, but also against downstream and your users.

I hope that you reconsider. You may not have been aware about the consequences of such a move, but now you are.

@timothyschoen I'm going to release a new pd-lua version as quickly as I can, promised. I hope that you understand that this needs a little time so that I can test the new version and make sure that it doesn't introduce any regressions. In the meantime, I can merge this PR asap so that you can already update the pd-lua submodule in plugdata if you want. But ultimately it's up to you to decide whether you prefer to include that in plugdata or your own fork (or Alexandre's).

agraef commented 6 months ago

Merged. @timothyschoen Thanks for the awesome contribution!

I just merged as is, then updated the doc and help patch, added your sig iolet example and Ben's osci3d~ example, and last but not least made it work with Purr Data again (dummy graphics for now, but the signal iolets work fine there).

It would be nice if you could pull this over into plugdata and your fork, so that you have my latest fixes.

I can hopefully do some more testing and then put out a release over the weekend, or maybe early next week. An overhaul of the tutorial is now in order as well, but that's for later. ;-)

porres commented 6 months ago

We already had this discussion the last time

Not really. Whatever you're bringing up here was never discussed, or you can show it where. Here's what I have to show https://github.com/porres/pd-else/issues/1477 ...

when you shipped an out-of-date and broken version of pd-lua with ELSE and refused to update it.

Also very incorrect and wrong. You can only be talking about the issue I just linked, where you asked me to update pdlua. So what really happened as we can see it there?

Show me, where did I "refuse" to do anything there? I'm sorry, but you're putting me in a very delicate position with these kinds of false accusations and you force me to defend myself and my honor in a public space. That's not the first time by the way where exchanging messages with you goes that way and we don't discuss what is important and I have to spend time and energy doing something like this... (I can show more references if needed).

So, what actually happened in this episode is that I was on a summer vacation, about a year ago, with my girlfriend, and wasn't available for like A WEEK to respond to a few (but annoyingly insistent) messages on more than one github issues (I'm not even referencing the others). Now, you know the English language well! This is quite different from 'denying', or 'refusing' to update, right? And to get on with what actually happened, within a week of your request I was there and we sorted things out!

More context and facts. You never bothered to offer any of your pdlua updates into deken for Vanilla users. I was including it in ELSE and doing this, for you... and all of a sudden me not immediately updating ELSE to carry the latest version into deken was such a serious problem... even if this would disrupt my update cycle which was even just a couple of weeks away. You couldn't wait 2 weeks! I said just wait a bit and it wasn't good for you, and now you come and say "I refused to update", which is a very distorted way of putting things, right? And that doesn't make me look good either, huh? :) it also makes you look like if you ask me to do something, you're not really asking, but you're giving me and 'order', and if I did not obey immediately and asked for like two weeks, I'm refusing to obey or something :)

And I did upload the pdlua package for you up in deken right away, didn't I? The updated one! It's still there, isn't it? I also put myself available to keep an eye on it and always upload the most recent updated version ASAP, didn't I? That's me being someone you need to talk to like that and say I "refused" to do things, huh? You know, I did this so you'd be happy and off my back... so we'd never have to go through with something similar, like you telling me to force an immediate update that can't wait DAYS to happen... and what a funny thing Albert, pdlua was never updated again in more than a year after this... so I guess the concern of keeping it up to date immediately and never wait (hell forbid) for like two weeks could be something to be managed back then?

Now hey, look at this, we finally have updates coming, from our end! And I'm also helping by the way. Like I helped before with the docs and uploading it and stuff. And this is funny too, cause here we have you taking as long as MONTHS to have a look at a PR, and then saying it should take many more months before you could make this release happen, when this is already part of a PugData release and things are broken, out of date and sync. How ironic, isn't it? Should we say you were 'refusing to update' as well at this moment?

Oh boy, there we go again.

Yep, we're back at it again, back at you being abusive, that's it :) look at this kind of disrespectful language... "oh boy"... amazing how you can be so unpleasant with just a few words... we're not that close buddies and I don't care for this kind of attitude. You may not have been aware before, but now you are. So please, try and adjust your tone. Try to respect others if you want respect back.

Only this time it's actually worse as you're removing features from pd-lua so your fork won't even be compatible.

Again, I've never done anything bad for you to compare how 'worse' I'd be behaving now... and I have to say we are not very sure on what your problem really is, and I'd hope you could give it another go and try to be less aggressive about it. Can you try without any personal attack to make it clear what it is that you're concerned about? If you can do this politely, we'd be glad to discuss it over with respect. Don't get out of your way to attack me please, cause it'll disrupt the discussion and force me to defend myself with actual facts, ok?

And to get on with things, maybe it wasn't clear that I'm not calling it 'pdlua' and telling people it is 'pdlua'... I thought it was clear when I said this would be a "modified version". You know, like Purr Data isn't really Pure Data? So have that in mind if that was a source of confusion. If that was perfectly clear, please just try again anyway.

Thanks and Cheers

ps. If you care so much about offering up to date and compatible packages, please consider updating Cyclone to the latest version (0.8-1 currently). Both Pd-l2ork and Purr Data still offer very outdated versions from Pd-Extended... (0.1-alpha56 or something), and the GUI port isn't yet complete for all objects. By the way, I removed [comment] from the documentation and made it deprecated, you don't even have to port that one, which you simply removed from the package. You see, you're not offering people the choice of which version of Cyclone to use, and forcing that old one down people's throat by bundling it with Purr Data is not what I would call "hostile", but "not good" in general. And you can see I've offered many times to help with this as an upstream developer... should I just start saying you're all refusing to update?

agraef commented 6 months ago

@porres Let's lay this to rest. I appreciate your contributions, I really do. I was worried, though, that people using ELSE might not be able to run the upstream version of pd-lua along with it. If that is still possible, then I'm fine with it.

porres commented 6 months ago

That's a valid concern and I had tested it without any issues, so far. I also discussed this previously with @timothyschoen saying we should care about that and asked him to help to make it work well for sure. He said he didn't see any problems with it. So don't worry about that.

And I hope, like I said first, that we can play back and forth and make things compatible and help each other.

I also hope you understand my concern in offering a streamlined version of this that works pretty nicely and easily for ELSE users in Vanilla. I care about that a lot.

I also got in touch with Claude about this and he's also fine with it all, by the way.

cheers

agraef commented 6 months ago

Ok, if the two variants can peacefully coexist then all is good. I must have had a really bad day when writing that reply, sorry about that. I do consider ELSE an awesome piece of work, pulling all that stuff together under one umbrella so that it becomes easy and accessible for all Pd users, that's very useful. I'd really like to have it in Purr Data at some point as well, but alas, we're not there yet.

agraef commented 6 months ago

@timothyschoen (Sorry, I just realized that I have no pm of you, so I'm contacting you this way.)

Do you know a student (or non-student) who might be interested in working on this (pd-lua graphics) during Google Summer of Code 2024, and earn a GSoC certificate and some money along the way? The main goal would be to port the gfx interface to Purr Data, of course, but it could also be something else related to the api.

If you know someone, please get in touch! (Mail me at aggraef - at - gmail.com.)