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

Loading screen: Be more verbose about what is happening. #525

Merged
merged 1 commit into from Feb 8, 2019

Conversation

Projects
None yet
5 participants
@lubosz
Copy link

lubosz commented Jan 31, 2019

Starting up emulationstation takes me about 1 minute over the network
with a large collection of 27 systems with images.

This patch uses the loading screen to tell the user about the status
of the startup, with information how many systems are left for view
initialization.

The most beefy part of the startup process is initializing the views,
and preloading images.

This patch extends the renderLoadingScreen function to take a string
and uses it in ViewController::preload.

@lubosz lubosz force-pushed the lubosz:loading-screen branch 2 times, most recently from cc5ea80 to 3d0dbb6 Jan 31, 2019

@pjft

This comment has been minimized.

Copy link

pjft commented Feb 1, 2019

Thank you for taking the time, this is a nice addition.
I'm wondering if it should be the default or an optional behavior.

I'll wait for others to chime in.

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Feb 1, 2019

I like it but I think it should be configurable also. Happy for it to default to on though.

Not tested yet. Thanks for the contribution.

@HerbFargus

This comment has been minimized.

Copy link
Member

HerbFargus commented Feb 1, 2019

I prefer a minimalist setup so I'd prefer it be optional but I'll defer to people who use it more than me.

@hhromic

This comment has been minimized.

Copy link

hhromic commented Feb 1, 2019

This is a really nice idea indeed, who doesn't like to see progress, specially when things take a while?
And I also agree that this should be a selectable option by the end-user.

However, I think the implementation is not efficient and can potentially add more time to the boot-up. The reason is because the entire loading screen is re-rendered after each text update. This seems to be done for a lot of objects, I guess for every graphic asset to load.

I would suggest instead to separate the main screen/logo rendering from the text rendering into individual methods, and then just call the latter only on text updates while loading. For this to work I think a quick "clearing" snippet should be also added on the text renderer so the new text doesn't overlap with the old one.
Also, the "Done." final update I'm not sure is necessary as it should move on as fast as possible.

Hope this helps!

@lubosz

This comment has been minimized.

Copy link
Author

lubosz commented Feb 4, 2019

This is a really nice idea indeed, who doesn't like to see progress, specially when things take a while?
And I also agree that this should be a selectable option by the end-user.

I agree that this can be an option.

However, I think the implementation is not efficient and can potentially add more time to the boot-up. The reason is because the entire loading screen is re-rendered after each text update. This seems to be done for a lot of objects, I guess for every graphic asset to load.

This is not correct. The screen is only re-rendered for each system, so 27 draw calls in 1 minute, which is basically idling on every GPU.

I would suggest instead to separate the main screen/logo rendering from the text rendering into individual methods, and then just call the latter only on text updates while loading. For this to work I think a quick "clearing" snippet should be also added on the text renderer so the new text doesn't overlap with the old one.

In my initial implementation I was just re-endering the text. Since the screen is cleared every draw call, only the text was drawn this way. The performance gain of clearing the screen partially and rendering the text only would not be measurable.

Also, the "Done." final update I'm not sure is necessary as it should move on as fast as possible.

Rendering of the done text has absolutely no impact on the completion time.

Hope this helps!

@hhromic

This comment has been minimized.

Copy link

hhromic commented Feb 4, 2019

This is not correct. The screen is only re-rendered for each system, so 27 draw calls in 1 minute, which is basically idling on every GPU.

You are right! Apologies, I overlooked the loop and thought the pre-loading was more granular (iterate over each graphical asset) but as you say, actually it iterates only per system. So, yes, I guess it's not that bad to re-draw the screen every time :)

Rendering of the done text has absolutely no impact on the completion time.

I assumed that after pre-loading, the startup moves forward to the GUI so fast that you actually don't get to see the "Done" frame at all, so why render it. But if the pre-loading takes a time between finishing and transitioning to the GUI, then I guess it's okay to render that frame.

@lubosz lubosz force-pushed the lubosz:loading-screen branch from 3d0dbb6 to e42df61 Feb 4, 2019

@lubosz

This comment has been minimized.

Copy link
Author

lubosz commented Feb 4, 2019

I assumed that after preloading the startup moves forward to the GUI so fast that you actually don't get to see the "Done" frame at all, so why render it. But if the preloading takes a time between finishing and transitioning to the GUI, then I guess it's okay to render that frame.

I guess you could argue that the text is visible so briefly that it could be removed for user experience reasons. I actually liked the progress text to say a last thing before ES appears.

The Done text is visible while the following code executes, which takes me about half a second on an i7, so it could take longer on embedded.

	//choose which GUI to open depending on if an input configuration already exists
	if(errorMsg == NULL)
	{
		if(Utils::FileSystem::exists(InputManager::getConfigPath()) && InputManager::getInstance()->getNumConfiguredDevices() > 0)
		{
			ViewController::get()->goToStart();
		}else{
			window.pushGui(new GuiDetectDevice(&window, true, [] { ViewController::get()->goToStart(); }));
		}
	}
Loading screen: Be more verbose about what is happening.
Starting up emulationstation takes me about 1 minute over the network
with a large collection of 27 systems with images.

This patch uses the loading screen to tell the user about the status
of the startup, with information how many systems are left for view
initialization.

The most beefy part of the startup process is initializing the views,
and preloading images.

This patch extends the `renderLoadingScreen` function to take a string
and uses it in `ViewController::preload`.

v2: Add SplashScreenProgress option enabled by default.

@lubosz lubosz force-pushed the lubosz:loading-screen branch from e42df61 to 87a3205 Feb 4, 2019

@hhromic

This comment has been minimized.

Copy link

hhromic commented Feb 4, 2019

@lubosz if that is the case, then surely is nice to have the final "Done" frame :)
Thanks for this nice addition! Hopefully makes it to the next stable release.

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Feb 8, 2019

Going to merge - thanks for this.

@joolswills joolswills merged commit 1246402 into RetroPie:master Feb 8, 2019

@joolswills

This comment has been minimized.

Copy link
Member

joolswills commented Feb 12, 2019

Thanks again for this. After some use on my systems I have noticed the "loading system config" actually takes a lot longer than the later more verbose progress later. Especially on one system that uses a network mount. I think it may be worth making the first step more verbose as it feels a little unbalanced. I may look into changing this a bit. PRs welcome though.

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment