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

pdlua graphics #35

Merged
merged 65 commits into from
Mar 1, 2024
Merged

pdlua graphics #35

merged 65 commits into from
Mar 1, 2024

Conversation

timothyschoen
Copy link
Contributor

@timothyschoen timothyschoen commented Dec 20, 2023

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

  • plugdata:

    • Drawing or filling rectangles/rounded rectangles/circles
    • Graphics transforms
    • Drawing vector paths
    • Mouse interaction
    • Correctly handles move/resize
    • Drawing lines
    • Drawing text
  • pure-data:

    • Drawing or filling rectangles/rounded rectangles/circles
    • Graphics transforms (kind of)
    • Drawing vector paths
    • Mouse interaction
    • Correctly handles move/resize
    • Drawing lines
    • Drawing text
  • purr-data:
    Not implemented yet, I was hoping someone with more purr-data experience could help out with that ;)

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
Copy link
Contributor Author

Screen.Recording.2023-12-20.at.21.45.22.mov

@agraef
Copy link
Owner

agraef commented Feb 28, 2024

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

@timothyschoen
Copy link
Contributor Author

timothyschoen commented Feb 28, 2024

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
Copy link
Owner

agraef commented Feb 29, 2024

@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
Copy link
Owner

agraef commented Feb 29, 2024

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
Copy link
Owner

agraef commented Feb 29, 2024

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.

@agraef
Copy link
Owner

agraef commented Feb 29, 2024

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

@timothyschoen
Copy link
Contributor Author

@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
Copy link
Contributor Author

timothyschoen commented Feb 29, 2024

@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
Copy link
Owner

agraef commented Feb 29, 2024

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
Copy link
Contributor

porres commented Feb 29, 2024

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
Copy link
Owner

agraef commented Feb 29, 2024

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 agraef merged commit 974bed0 into agraef:master Mar 1, 2024
4 checks passed
@agraef
Copy link
Owner

agraef commented Mar 1, 2024

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. ;-)

@agraef agraef mentioned this pull request Mar 1, 2024
@porres
Copy link
Contributor

porres commented Mar 11, 2024

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 porres/pd-else#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
Copy link
Owner

agraef commented Mar 11, 2024

@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
Copy link
Contributor

porres commented Mar 11, 2024

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
Copy link
Owner

agraef commented Mar 12, 2024

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
Copy link
Owner

agraef commented Mar 13, 2024

@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.)

agraef added a commit that referenced this pull request Sep 4, 2024
These were both part of PR #35 (Tim's graphics and signal
implementation). The changeset consisted of two unrelated parts:

- An attempt to work around defects in the menu open action.

- An ad-hoc implementation of object reloading functionality.

As the bugs in the menu open action have since been fixed, and the
pdx.lua live-coding extension already provides a way to get automatic
object reloading which improves over the method being used here in every
way, this code is now obsolete and can be removed.
agraef added a commit that referenced this pull request Sep 4, 2024
These were both part of PR #35 (Tim's graphics and signal
implementation). The changeset consisted of two unrelated parts:

- An attempt to work around defects in the menu open action.

- An ad-hoc implementation of object reloading functionality.

As the bugs in the menu open action have since been fixed, and the
pdx.lua live-coding extension already provides a way to get automatic
object reloading, this code is now obsolete and can be removed.
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.

4 participants