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

Splash Screen (#1416) #1420

Merged
merged 17 commits into from
Nov 17, 2022
Merged

Splash Screen (#1416) #1420

merged 17 commits into from
Nov 17, 2022

Conversation

wladimirleite
Copy link
Member

As discussed in #1416, I implemented a simple splash screen to be presented while IPED's analysis UI is starting up.

The third option mentioned in the issue (setting a static image in the application JAR manifest) worked fine, as it is presented before the JVM starts, so the splash screen is shown fast enough, even when opening cases from a very slow location.

The default behavior is to close the splash screen when the application opens a window. In this case, this doesn't work as there are 2 processes. The splash belongs to the main one (so it is presented quickly), but the analysis UI is created by a second process (child of the first one).

I implemented a simple communication between the two processes through a temporary file (that uses the second process pid as an unique identifier).
That communication allowed not only informing about the end of the start up phase (main window was created), but also providing some feedback about the progress of the process initialization.

About the progress, instead of placing in several parts of initialization code some call to update the progress, I used an heuristic that counted the number of loaded classes. That gave a rough estimate of the progress, without adding a burden to future changes in initialization related code.

I added an option, through a configuration file, to show an optional custom message in the splash window. That could be used to show the organization name or the case identification.

Finally, the UI design used in the splash screen is an initial version.
Things like colors, fonts, sizes, positions, shadows, and the image that was included are personal choices.
I tried to keep a somewhat clean design, as used in MS Office and other common Windows applications nowadays.
@lfcnassif, please feel free to suggest visual improvements or a completely new design.

@lfcnassif
Copy link
Member

About the progress, instead of placing in several parts of initialization code some call to update the progress, I used an heuristic that counted the number of loaded classes. That gave a rough estimate of the progress, without adding a burden to future changes in initialization related code.

Very cool, thank you @tc-wleite!

@lfcnassif
Copy link
Member

Just tried this, very good! I only need to think more about the logo...

@wladimirleite
Copy link
Member Author

I only need to think more about the logo...

@lfcnassif, commited an alternative design, using the search icon already used in the application.

Also added a command line option to set the splash screen custom message.
Just a minor suggestion.
We can add a test field with this option in the report generation dialog, if you think this option makes sense.

@lfcnassif
Copy link
Member

@lfcnassif, commited an alternative design, using the search icon already used in the application.

Thanks @tc-wleite. We also have this in PR #1341, maybe we could use a similar icon:
https://github.com/sepinf-inc/IPED/blob/fb3d4865211a1a82fe1c1188bd0a19bc5cb6e54c/iped-app/src/main/resources/iped/app/home/IPED-logo_lupa.png

Also added a command line option to set the splash screen custom message.
Just a minor suggestion.
We can add a test field with this option in the report generation dialog, if you think this option makes sense.

To be shown when opening reports? I liked the idea! A similar thing was asked a long ago by some users.

@wladimirleite
Copy link
Member Author

wladimirleite commented Nov 4, 2022

Thanks @tc-wleite. We also have this in PR #1341, maybe we could use a similar icon: https://github.com/sepinf-inc/IPED/blob/fb3d4865211a1a82fe1c1188bd0a19bc5cb6e54c/iped-app/src/main/resources/iped/app/home/IPED-logo_lupa.png

Cool! I only avoid put the IPED "full name", as it is in Portuguese and may sound/look odd for foreign users.

@thiagofuer
Copy link

Thanks @tc-wleite. We also have this in PR #1341, maybe we could use a similar icon: https://github.com/sepinf-inc/IPED/blob/fb3d4865211a1a82fe1c1188bd0a19bc5cb6e54c/iped-app/src/main/resources/iped/app/home/IPED-logo_lupa.png

Cool! I only avoid put the IPED "full name", as it is in Portuguese and may sound/look odd for foreign users.

We could remove text from logo image and translate in runtime.

@thiagofuer
Copy link

Thanks @tc-wleite. We also have this in PR #1341, maybe we could use a similar icon: https://github.com/sepinf-inc/IPED/blob/fb3d4865211a1a82fe1c1188bd0a19bc5cb6e54c/iped-app/src/main/resources/iped/app/home/IPED-logo_lupa.png

Cool! I only avoid put the IPED "full name", as it is in Portuguese and may sound/look odd for foreign users.

We could remove text from logo image and translate in runtime.

Or try to change logo to a "no text" version..
But no idea by now..

@wladimirleite
Copy link
Member Author

wladimirleite commented Nov 4, 2022

We can add a test field with this option in the report generation dialog, if you think this option makes sense.

To be shown when opening reports? I liked the idea! A similar thing was asked a long ago by some users.

Reports or any processed case.
I added an option to set a custom message in a configuration file.
This recent commit was to also add a command line option.
Something more complex could be added, but I tried to keep it simple, as the main goal of the splash screen was to present something while the application is opening, especially before the main window is visible, so the user knows that the program is actually running.

@wladimirleite
Copy link
Member Author

Added a third design option, using an image derived from the logo used in PR #1341.

@lfcnassif
Copy link
Member

lfcnassif commented Nov 4, 2022

Added a third design option, using an image derived from the logo used in PR #1341.

Thank you @tc-wleite! I improved it a bit further. I'm rolling an internal poll. Let's also vote here between this:
image

@lfcnassif
Copy link
Member

lfcnassif commented Nov 4, 2022

And this one:
image

@lfcnassif
Copy link
Member

Users are also welcome to vote!

@abdalla-mar
Copy link

And this one: image

Vote this one. However, could include a new icon (or replace the desktop one) for web data.

@wladimirleite
Copy link
Member Author

Replaced the CD by an Internet icon, as @abdalla-mar suggested and made minor visual adjustments (ticker icon lines, better text alignment).
Below is an example of it running, with a custom (optional) message.

image

@lfcnassif
Copy link
Member

lfcnassif commented Nov 4, 2022

Replaced the CD by an Internet icon, as @abdalla-mar suggested and made minor visual adjustments (ticker icon lines, better text alignment). Below is an example of it running, with a custom (optional) message.

image

This one is winning the internal poll by 10 votes against 4 (actually its previous version with CD icon).

@lfcnassif
Copy link
Member

An advantage of the first option is that the magnifier image could be reused as the new application icon, reinforcing the software visual identity.

@lfcnassif
Copy link
Member

Replaced the CD by an Internet icon, as @abdalla-mar suggested and made minor visual adjustments (ticker icon lines, better text alignment). Below is an example of it running, with a custom (optional) message.

image

This won an internal poll by 11 against 4 votes.

@lfcnassif
Copy link
Member

lfcnassif commented Nov 16, 2022

@tc-wleite I pushed some changes to also display the splash screen when processing a case. The following message was being printed on Console when running iped.exe:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by iped.app.ui.splash.StartUpControl (file:/E:/git/iped/target/release/iped-4.1-snapshot/iped.jar) to field java.lang.ClassLoader.classes
WARNING: Please consider reporting this to the maintainers of iped.app.ui.splash.StartUpControl
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release

I tried 2 alternative implementations for StartUpControl class, but I didn't manage to make them work properly. So I simply commented out the code to get the number of loaded classes from the first bootstrap process and adjusted the min/max progress values. Seems to be working fine to me.

I just need to test this on a non Screen Linux environment to check if the splash screen pop up would break processing on that scenario.

@wladimirleite
Copy link
Member Author

wladimirleite commented Nov 16, 2022

@tc-wleite I pushed some changes to also display the splash screen when processing a case. The following message was being printed on Console when running iped.exe:

I didn't see that error here, but I am not sure if I tested using the console.

I tried 2 alternative implementations for StartUpControl class, but I didn't manage to make them work properly. So I simply commented out the code to get the number of loaded classes from the first bootstrap process and adjusted the min/max progress values. Seems to be working fine to me.

Ok, that seems fine!
I tried other methods to get the number of loaded classes, but they didn't work properly.
Maybe there are other approaches, but I believe that your fixes are enough for now.

I just need to test this on a non Screen Linux environment to check if the splash screen pop up would break processing on that scenario.

I was expecting that SplashScreen.getSplashScreen() would return null, but reading the documentation now, I realized that it may throw an exception in such situations.
Just pushed a fix.
Anyway, it is important to test in a headless environment.

@lfcnassif
Copy link
Member

I was expecting that SplashScreen.getSplashScreen() would return null, but reading the documentation now, I realized that it may throw an exception in such situations.
Just pushed a fix.

I also thought this, thank you for the fix! I'm also concerned if the application may abort early before entering its main() method or if the JVM will ignore the new manifest entry...

@lfcnassif
Copy link
Member

Anyway, it is important to test in a headless environment.

Just tested, worked fine! Without using the --nogui option an expected exception is thrown:

java.awt.HeadlessException: 
No X11 DISPLAY variable was set, but this program performed an operation which requires it.

Using it, processing finished fine.

@lfcnassif lfcnassif merged commit 767dc4b into master Nov 17, 2022
@lfcnassif lfcnassif deleted the #1416_SplashScreen branch November 17, 2022 19:26
@lfcnassif
Copy link
Member

lfcnassif commented Nov 17, 2022

PS: Exception thrown is not related to the Splash but to JFrame creation, and it happens after the splash is shown, so the JVM ignores the splash screen.

@lfcnassif lfcnassif mentioned this pull request Nov 17, 2022
@lfcnassif
Copy link
Member

Just a final observation about this noticed by @arisjr, when running with --nogui option on an environment with a screen device, the splash is shown. I don't know if it is possible to workaround this, but I don't think it is a real problem anyway.

@wladimirleite
Copy link
Member Author

wladimirleite commented Jan 27, 2023

Just a final observation about this noticed by @arisjr, when running with --nogui option on an environment with a screen device, the splash is shown. I don't know if it is possible to workaround this, but I don't think it is a real problem anyway.

Forgot to reply this...
I don't think it is possible, as the splash screen is displayed quickly because it is set in the JAR manifest (and shown before the JVM start). But as you verified, it works in a headless environment, so I guess there is not much to be done.

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