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 "goprect" method to graphs for dynamically changing the GOP rect #627

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Spacechild1
Copy link
Contributor

@Spacechild1 Spacechild1 commented May 11, 2019

The purpose of this PR is mainly to stop people from abusing internal messages like [coords( - or even worse: [donecanvasdialog( - for dynamically adjusting the GOP rect. Instead I want to offer a dedicated message for this purpose: [goprect <xmargin> <ymargin> <width> <height>(.

Width and height are optional, e.g. [goprect 100 0( will just move the GOP rect to position (100, 0) without changing the dimensions. This can be used for dynamically showing different areas of a subpatch and implementing "tabbed widgets" for example.

[goprect( redraws the parent canvas to avoid graphical artifacts ("ghost objects"). With [coords( and [donecanvasdialog(, you need to do [map 0, map 1( to achieve the same thing.

Note that we already have a [bounds <x1> <y1> <x2> <y2>( message (see 2.control.examples/16.more.arrays.pd) which sets the X/Y range in GOP patches (or the X/Y units in ordinary patches), so [goprect( would just be the missing piece.

Just like [bounds(, [goprect( doesn't affect the undo queue.

See canvas-help.pd for examples.

@LGoodacre
Copy link

Adding new dynamic patching capabilities is great, but could you please elaborate on what would be gained from breaking backwards compatibility with coords and donecanvasdialogue? Why exactly are they considered "abuse"?

@Spacechild1
Copy link
Contributor Author

Spacechild1 commented Jun 8, 2019

from breaking backwards compatibility with coords and donecanvasdialogue?

Just to be clear, I'm not trying to remove those messages (after all, they are internally used by Pd). We can still try to keep old patches working but at the same time recommend updating to [goprect( for better stability.

Why exactly are they considered "abuse"?

In a way, [coords( is less evil because it's used in Pd patch files and its behavior is more stable. On the other hand, [donecanvasdialog( is a reply message sent from a GUI dialog and certainly not meant to be called by anyone else (I'm guilty as charged :-). It's behavior is not consistent across Pd versions and it's quite easy to crash Pd in mysterious ways.

What I'm offering here is a stable and practical alternative for manipulating the GOP rect. With [bounds( we already have a documented and supported message to manipulate the array's x/y range; [goprect( only deals with the GOP size/position. It's not only safer but also much easier to use!

@danomatika
Copy link
Contributor

could you please elaborate on what would be gained from breaking backwards compatibility with coords and donecanvasdialogue?

The ability to add/change the values sent to donecanvasdialog for future GUI updates without breaking everyone's (currently unsupported) dynamic patching behavior?

@Spacechild1
Copy link
Contributor Author

@millerpuckette just in case you feel like sneaking a small feature into Pd 0.52.3 ;-). We keep having trouble with people (ab)using [donecanvasdialog( and [coords( and consequently complaining about various "bugs". A proper public message like this could finally solve it.

@porres
Copy link
Contributor

porres commented Nov 26, 2021

what about setting it to graph on parent, and show/hide object name and arguments?

@Spacechild1
Copy link
Contributor Author

what about setting it to graph on parent, and show/hide object name and arguments?

Is there ever a situation where you wanted to this dynamically? Generally, we can always add arguments (later).

@porres
Copy link
Contributor

porres commented Nov 26, 2021

I've been using, in else/sample I have a [pd $0-buffers] subpatch and I create arrays to display audio from each channel dynamically. The way I know how to do this is how I tried to document today in my branch:

create a subpatch with [pd 'x'] and tell it to graph on parent hiding arg an name so I can then include an array in it.

Now, we could add a graph, but we can't choose the x/y coordinates of the graph this way, unlike 'obj' message. Maybe that's an alternative... add this possibility, then resize it with this new message.

@jmmmp
Copy link

jmmmp commented Nov 26, 2021

The PR's purpose stated at the start isn't really achieved - at least in this page I didn't see any mention of the parameters GOP appearance, and all the parameters related to structs: X/Y units, X/Y range. Or are these covered by another new message? If not, the only option for dynamic changes is to use the old messages.

@Spacechild1
Copy link
Contributor Author

Spacechild1 commented Nov 26, 2021

parameters GOP appearance

do you mean "hide object name and arguments"? I never needed to change that dynamically. Either way, I think it should be another message, as it has nothing to do with the GOP rectangle.

all the parameters related to structs: X/Y units, X/Y range.

There is already the [bounds( message which sets the X/Y range in GOP patches resp. the X/Y units in non-GOP patches. I should mention this in the PR description (EDIT: done)

@jmmmp
Copy link

jmmmp commented Nov 26, 2021

parameters GOP appearance

do you mean "hide object name and arguments"? I never needed to change that dynamically. Either way, think it should be another message, as it has nothing to do with the GOP rectangle.

I use it in [jmmmp/multiarray], and also in several own patches but exceptionally. It's useful when redoing complex guis and save tcl/tk bottlenecks.

@porres
Copy link
Contributor

porres commented Nov 27, 2021

I use it in [jmmmp/multiarray], and also in several own patches but exceptionally. It's useful when redoing complex guis and save tcl/tk bottlenecks.

can you give us a specific example/reason why?

@Spacechild1
Copy link
Contributor Author

I use it in [jmmmp/multiarray], and also in several own patches but exceptionally. It's useful when redoing complex guis and save tcl/tk bottlenecks.

That's a nice trick! So it seems what you actually want is a way to temporarily disable sending GUI commands. I think you should ask for such a feature then. I don't think it makes sense to replace one hack with another, let's fix the actual issue instead!

@jmmmp
Copy link

jmmmp commented Nov 27, 2021

That's a nice trick! So it seems what you actually want is a way to temporarily disable sending GUI commands. I think you should ask for such a feature then. I don't think it makes sense to replace one hack with another, let's fix the actual issue instead!

well, the real issue would be for the gui rendering to be more efficient, specially with arrays. everything else is a hack around it, be it an "official" or "unofficial" one. but this is a topic as long as Pd exists, so I think many minds have thought about this.

@Spacechild1
Copy link
Contributor Author

well, the real issue would be for the gui rendering to be more efficient, specially with arrays.

That's true. GUI performance in Pd is horrible. Many things are getting redrawn all the time even if they don't need to be. But let's move this discussion somewhere else, e. g. the mailing list.

porres added a commit to porres/pure-data that referenced this pull request Dec 1, 2021
removed 'coords' message from documentation, after discussion on the list, idea is to wait for  pure-data#627 and document the new proposed message instead
@danomatika
Copy link
Contributor

Many things are getting redrawn all the time even if they don't need to be.

Note: This could be partially rectified by using the tkcanvas move method which was introduced in Tk 8.5, then Pd could simply move existing elements instead of destroying and creating them. This is something I'd like to look into now that we are firmly past Tk 8.4 where the method was not available and could also replace a good amount of redundant core messaging.

@Spacechild1
Copy link
Contributor Author

If there is some interest in merging, I will fix the conflicts.

@millerpuckette
Copy link
Contributor

I need to figure out how to fix number/symbol/list boxes within GOPs before adding any new functionality... maybe that's the next thing I should be working on since it's pretty badly broken.

@Spacechild1
Copy link
Contributor Author

I need to figure out how to fix number/symbol/list boxes within GOPs before adding any new functionality...

Hmmm... I'm not sure how these two things are related. Also, this PR does not really add any "new" functionality. It has always been possible to change the GOP rect, but only with a dialog. Now we would be able to do it with a message. That's really it.

@millerpuckette
Copy link
Contributor

Only in that fixing the GOP trouble will require a massive rewrite and probably everything about GOPs will have to be revisited.

@Spacechild1
Copy link
Contributor Author

Only in that fixing the GOP trouble will require a massive rewrite

Oh...

probably everything about GOPs will have to be revisited.

AFAICT, the GOP mechanism itself can't really change without breaking virtually every existing patch. (Think of the coords message in patch files.) It's just the implementation that could be improved.

So as long as GOPs stay rectangular, nothing would really change for this PR. But if you are not even sure about that, than better wait :-)

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

Successfully merging this pull request may close these issues.

Objects don't disappear after changing GOP settings
7 participants