Bug 426243 - [All] Add Display API to spin event loop
Summary: [All] Add Display API to spin event loop
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.8   Edit
Hardware: PC All
: P5 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-SWT-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted, triaged
Depends on:
Blocks:
 
Reported: 2014-01-21 08:08 EST by Thomas Schindl CLA
Modified: 2018-07-10 14:14 EDT (History)
4 users (show)

See Also:


Attachments
Cocoa implementation (2.44 KB, patch)
2014-01-21 08:11 EST, Thomas Schindl CLA
no flags Details | Diff
Revise patch upon Pauls requirement (3.21 KB, patch)
2014-01-21 09:05 EST, Thomas Schindl CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2014-01-21 08:08:36 EST
For toolkits who don't offer an event loop in the way SWT does where you often write:

while( <condition> ) {
  if( d.readAndDispatch() ) {
    d.sleep();
  }
}

it would be nice to get an API like block(BlockCondition) which internally can use the ports preferred way of spinning the event loop.
Comment 1 Thomas Schindl CLA 2014-01-21 08:11:43 EST
Created attachment 239179 [details]
Cocoa implementation

This patch holds:
* the requested API for Display
* an additional API for Shell to open it blocking (not strictly necessary)

If you agree with the API I'd provide implementations for all platforms
Comment 2 Thomas Schindl CLA 2014-01-21 08:23:47 EST
https://git.eclipse.org/r/20867
Comment 3 Paul Webster CLA 2014-01-21 08:41:19 EST
Tom, how can we handle this case.  The while part can be turned into the Condition (and the spinOnce+return.  But we have a requirement to execute a method on eventLoopIdle (our default implementation has the display.sleep() in it, our IDE implementation has a delayedEventsProcessor call before the display.sleep())


while (((testShell != null && !testShell.isDisposed()) || (theApp != null && someAreVisible(theApp
		.getChildren()))) && !display.isDisposed()) {
	try {
		if (!display.readAndDispatch()) {
			runContext.processWaiting();
			if (spinOnce)
				return;
			advisor.eventLoopIdle(display);
		}
	} catch (ThreadDeath th) {
		throw th;
	} catch (Exception ex) {
		handle(ex, advisor);
	} catch (Error err) {
		handle(err, advisor);
	}
}

PW
Comment 4 Thomas Schindl CLA 2014-01-21 09:05:08 EST
Created attachment 239182 [details]
Revise patch upon Pauls requirement

Paul what about that?
Comment 5 Lars Vogel CLA 2014-01-21 09:09:03 EST
(In reply to Thomas Schindl from comment #4)
> Created attachment 239182 [details]
> Revise patch upon Pauls requirement
> 
> Paul what about that?

Could you update the Gerrit review? Just amend you last commit and push again.
Comment 6 Leo Ufimtsev CLA 2017-06-26 12:53:29 EDT
@Thomas, 

I like the concept that you introduced. A one-liner that replaces the usual while<> loop.

If I understand correctly, we want to replace this pattern:
	while (!shell.isDisposed()) {
		if (!display.readAndDispatch ()) display.sleep ();
	}

With:
	block(shell); // 1*

A few questions:
- What name do you suggest for 1*? block()?
- If I understand correctly, the api should only be provided in Display by a single method. The patch introduces a bunch of different public methods. Can it be done in such a way that only one method is made public?

Considering the old-age of the bug. Do you still intend to work on this bug?
Comment 7 Thomas Schindl CLA 2017-07-03 03:03:51 EDT
(In reply to Leo Ufimtsev from comment #6)
> @Thomas, 
> 
> I like the concept that you introduced. A one-liner that replaces the usual
> while<> loop.
> 
> If I understand correctly, we want to replace this pattern:
> 	while (!shell.isDisposed()) {
> 		if (!display.readAndDispatch ()) display.sleep ();
> 	}
> 
> With:
> 	block(shell); // 1*
> 

Please see Pauls usecase as well so it was renamed into spinEventLoop.

> A few questions:
> - What name do you suggest for 1*? block()?
> - If I understand correctly, the api should only be provided in Display by a
> single method. The patch introduces a bunch of different public methods. Can
> it be done in such a way that only one method is made public?
> 

It adds 1 method to display and 1 to Shell because the spinEventLoop useage is a bit more sophisticated.

> Considering the old-age of the bug. Do you still intend to work on this bug?

Not really because I abandoned my work for SWTonJavaFX.
Comment 8 Leo Ufimtsev CLA 2017-07-03 11:53:43 EDT
(In reply to Thomas Schindl from comment #7)

I see. Thanks for reply. 

I think the implementation with Runnable and listeners is nice, but feels somewhat over complicated. Like it seems like it would make users add more code than just using the standard event loop mechanism. (Thoughts? I may be wrong. I haven't looked into it too deeply.). 

I'd be in favor a solution that would:
1) Just add a one-liner to replace the common pattern.
2) Use standard event loop mechanism for more complicated scenario
3) Add some snippets to demonstrate both cases. (maybe update existing snippets with one liner, add a new snippet for manual event loop mechanism).


(In reply to Thomas Schindl from comment #7)
> > Considering the old-age of the bug. Do you still intend to work on this bug?
> 
> Not really because I abandoned my work for SWTonJavaFX.

Atm, no one is working on this bug, so I'm adding "Helpwanted" & setting priority to "helpwanted" level (see 1). Anyone is welcome to submit feedback/patches, I'd be happy to review.
[1] https://wiki.eclipse.org/WTP/Conventions_of_bug_priority_and_severity