Opened 12 years ago

Closed 10 years ago

#9159 closed bug (fixed)

PatchBay not showing tooltips

Reported by: Pete Owned by: phoudoin
Priority: normal Milestone: R1
Component: Applications Version: R1/Development
Keywords: Cc: ajcsweb@…
Blocked By: #9604 Blocking:
Platform: All

Description

Just noticed that PatchBay was no longer showing the tooltips with the names of the endpoints -- a bit important, as it's the only way of identifying the icons!

This seems to be due to a subtle change in the way a BWindow run loop, and a sequence of Hide/Show calls, is handled. Basically there is a redundant initial Hide() that used not to matter, but now prevented the window from ever showing. The attached patch fixes the problem.

(Patchbay isn't in the list of Apps for specifying the component. It probably ought to be -- and be included in the packaged Apps.)

Attachments (1)

patchbay.diff (829 bytes ) - added by Pete 12 years ago.
patch to make tooltips work again

Download all attachments as: .zip

Change History (19)

by Pete, 12 years ago

Attachment: patchbay.diff added

patch to make tooltips work again

comment:1 by Pete, 12 years ago

patch: 01

comment:2 by axeld, 12 years ago

Resolution: invalid
Status: newclosed

Are you on the right bug tracker?

In any case, an extra Hide() already mattered on BeOS, too; that's how for example Tracker hides its status window. Are you sure that this is the actual issue here? Could just be a non initialized fTip.showing, too.

Last edited 12 years ago by axeld (previous) (diff)

in reply to:  2 ; comment:3 by Pete, 12 years ago

Replying to axeld:

Are you on the right bug tracker?

Well, PatchBay is supposed to be a system app, no? This is certainly where the earlier patches to PatchBay were posted.

In any case, an extra Hide() already mattered on BeOS, too; that's how for example Tracker hides its status window. Are you sure that this is the actual issue here? Could just be a non initialized fTip.showing, too.

Yes, I know that... The point is that there alway was an initial Hide (before the first show) that got ignored in BeOS and previously in Haiku. It no longer is, so I've made sure with the patch that it is not called unnecessarily.

My interest is to get audio and midi to a point in Haiku where it actually works. This patch fixes a problem, and needs to be applied. Why do you want to block that (and other ways I've been trying to help)?

comment:4 by Pete, 12 years ago

Resolution: invalid
Status: closedreopened

in reply to:  3 comment:5 by Pete, 12 years ago

Replying to Pete:

Replying to axeld:

Are you on the right bug tracker?

Well, PatchBay is supposed to be a system app, no? This is certainly where the earlier patches to PatchBay were posted.

OK, I stand -- slightly -- corrected. I guess PatchBay has never really been part of the system. I'd forgotten it was originally BeOS sample code, because I've had it handy forever. But it is in the Haiku source tree, because Philippe put it there so that it would be available (see ticket #4562 which is where I remember discussing it). It's placed under "tests", though, so not all that visible.

It does need to be accessible, for those who need it. I guess as an optional package? Where are bugs in those ticketed?

comment:6 by axeld, 12 years ago

Sorry Pete, I did not realize that PatchBay is even part of our repository.

If the Hide() is an actual problem, you should open a ticket for that -- Ryan recently changed some code in that regard, and there is definitely the possibility that there are some regressions.

If you think PatchBay is useful, feel free to create an optional package for it. If you intend to spend more time on it, replacing its home-brewn tool tip with Haiku's native support would also be an option :-)

in reply to:  6 ; comment:7 by Pete, 12 years ago

Replying to axeld:

Sorry Pete, I did not realize that PatchBay is even part of our repository.

Yeah, I find it sort of ironic that Haiku has the most advanced and flexible MIDI handling of any OS I've come across, but it's pretty much the ignored child... (:-/)

If the Hide() is an actual problem, you should open a ticket for that -- Ryan recently changed some code in that regard, and there is definitely the possibility that there are some regressions.

No, I doubt that it would ever be a general problem. As Philippe noted in that other ticket, the author breaks a few BeOS rules. [And I assume he worked for Be!] In particular, he starts the Tooltip window with a Run(), rather than a Show() as specified in the BeBook, because he needs the Looper running before ever showing the window. (It's a message that displays the window.) I suspect there was a flag in the original code that prevented a Hide before the first Show being effective. Ryan probably changed that, but I'd guess it's not likely to affect anyone who obeys the rules!

I left the Run() in, though I suppose it might break with a later rev I found a Show()/Hide() pair in place of it also works without a visible glitch, but it seemed even uglier...

If you think PatchBay is useful, feel free to create an optional package for it. If you intend to spend more time on it, replacing its home-brewn tool tip with Haiku's native support would also be an option :-)

OK, I'll look into making an optional package. (Looking at the howto on the website, I may have a question or two, but I'll post them to the mailing list.)

At some point, it probably would be worth a bit of a rewrite, but at the moment I'm much more interested in other things... I've just got the latest Csound [well, about one release back, actually] playing live quite nicely. Now I need to redo it for the latest release and get it tidied up into a distributable package.

in reply to:  7 comment:8 by mmadia, 12 years ago

Replying to Pete:

Replying to axeld:

If you think PatchBay is useful, feel free to create an optional package for it. If you intend to spend more time on it, replacing its home-brewn tool tip with Haiku's native support would also be an option :-)

OK, I'll look into making an optional package. (Looking at the howto on the website, I may have a question or two, but I'll post them to the mailing list.)

Since this is part of Haiku's source tree, you should look at how Bluetooth is added as an optional package. http://cgit.haiku-os.org/haiku/tree/build/jam/OptionalPackages#n314

comment:9 by leavengood, 12 years ago

If you think it is important to perfectly replicate BeOS behavior I could test this particular scenario on BeOS and fix our Hide()/Show() code to match BeOS. Though I think there was some stupidity there that I chose to change, maybe this was part of it. It has been a while though, so I'd have to refresh my memory.

But if this was the only thing affected and that small patch fixes the problem, I probably should not waste the time.

in reply to:  description comment:10 by waddlesplash, 12 years ago

Cc: ajcsweb@… added

Replying to Pete:

(Patchbay isn't in the list of Apps for specifying the component. It probably ought to be -- and be included in the packaged Apps.)

Since it's not, I don't think anyone will benefit from this patch. So until this goes onto the Optional Packages or into the main distro, this patch is probably pointless. So can we add it to the list of apps? Or as an optional package?

comment:11 by phoudoin, 12 years ago

PatchBay being to MIDI Kit what Cortex is to Media Kit, I'm for promoting it to an builtin app.

Which leads to create first an enhancement/bug ticket to fix/rewrote it to conform to our 2013's code quality standard :-).

Version 0, edited 12 years ago by phoudoin (next)

in reply to:  11 comment:12 by Pete, 12 years ago

Replying to phoudoin:

PatchBay being to MIDI Kit what Cortex is to Media Kit, I'm for promoting it to an builtin app.

+1

Which leads to create first an enhancement/bug ticket to fix/rewrote this app to conform to our 2013's code quality standard :-).

I'll take a crack at it. I'll fix guideline issues anyway (and hope I get them right...!) Not sure how much if any rewriting is needed, though I guess the custom tooltip code shold be converted to standard Haiku.

comment:13 by anevilyak, 12 years ago

Owner: changed from nobody to phoudoin
Status: reopenedassigned
Version: R1/alpha4R1/Development

Assigning to phoudoin for future patch review since he's most familiar with the midi kit.

in reply to:  11 comment:14 by Pete, 12 years ago

Replying to phoudoin:

PatchBay being to MIDI Kit what Cortex is to Media Kit, I'm for promoting it to an builtin app.

Which leads to create first an enhancement/bug ticket to fix/rewrote this app to conform to our 2013's code quality standard :-).

OK -- following your request, I've created enhancement ticket #9604.

Aside from hopefully bringing it up to standard, I've changed it to use Haiku ToolTips, and added some icons. Otherwise the structure is pretty much unchanged.

comment:15 by phoudoin, 12 years ago

Blocking: 9604 added

comment:16 by phoudoin, 12 years ago

Blocked By: 9604 added
Blocking: 9604 removed

comment:17 by Pete, 11 years ago

Should this ticket not just be closed now? #9604 was created to hold the revisions which include fixing this problem, and that patch has been committed.

comment:18 by pulkomandy, 10 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.