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

Redesign the login screen #2366

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Conversation

TOMATO-ONE
Copy link
Contributor

@TOMATO-ONE TOMATO-ONE commented Sep 13, 2022

See Discussions #2342.

I have created a new login screen screen for use in the next major version.

New logo created by @metalefty.
Background colour taken from Windows 10 RDP login screen.
Login screen colours taken from Windows 10 mstsc.exe.

Add bmp logo image sized for HiDPI login screen and transparent png image for imlib2 build.

image

@TOMATO-ONE TOMATO-ONE force-pushed the new_loginscreen branch 5 times, most recently from 7442255 to 07c48c7 Compare September 13, 2022 14:43
@matt335672
Copy link
Member

Hi @TOMATO-ONE

Thanks very much for doing this.

Here are my initial thoughts:-

  1. The files fontutils/xrdp-dumpfv1 and xrdpapi/xrdp-xrdpapi-simple are temporary files generated by libtool and should be removed.
  2. In xrdp/xrdp.ini.in, where you have commented out old values and added new ones, I suggest you just change the values to the new ones.
  3. in xrdp/xrdp.ini.in, use keep the empty value for the ls_logo_filename and work out the default as xrdp_logo_1.bmp or xrdp_logo_1.png using the code in this comment as a guide. This makes it easier for packagers to build with imlib2 as they don't have to modify the init file to get the png file loaded.
  4. You can make xrdp_logo_1.bmp file considerably smaller by using an indexed bitmap. According to the GIMP the image you've presented uses 386 colours. If you're prepared to go down to 256 colours, you can get an image about a third of the size. Here's a zip of a suitable file for you to try:-
    xrdp_logo_1_8bit.zip
    The xrdp loader code also supports 16-colour bitmaps, but that's only 16 colours:-
    xrdp_logo_1_4bit.zip

Let me know if that's useful or if you have any questions.

@metalefty
Copy link
Member

The files fontutils/xrdp-dumpfv1 and xrdpapi/xrdp-xrdpapi-simple are temporary files generated by libtool and should be removed.

We might need to add them to gitignore.

@TOMATO-ONE
Copy link
Contributor Author

TOMATO-ONE commented Sep 15, 2022

Thank you.

@matt335672
About 1. The addition of these files was not my intention. It has already been addressed by @metalefty and will be removed from my PR.

Regarding 2. I stand by your opinion. I will not keep the previous parameter values.

Regarding 3. You mean to change the logo file name hard-coded on the source code side to xrdp_logo_1.bmp / xrdp_logo_1.png.
An alternative would be to not change the source code, but to rename the new logo image file to xrdp_logo.bmp. In this case, the current logo image will be replaced. Should we keep the current logo image for backward compatibility?

Regarding 4, I did not know that indexed bitmap saves file size. I took a PNG image exported by inkspace and simply opened it in Microsoft Paint and converted it to a BMP image. Let me use the 4bit BMP you converted in Gimp.

@matt335672
Copy link
Member

Hi @TOMATO-ONE

The reasoning behind changing the source code is that if the code is build with imlib2, the png is loaded instead. This has the small benefit that transparency is included for free, so if the user starts to change the colours they don't have to edit the logo as well.

I quite like the idea of keeping the old logo about - it's been done in the past.

@TOMATO-ONE
Copy link
Contributor Author

@matt335672
Thank you for your reply.

I accept your suggestion.

I have carried out the following

  • Replace xrdp_logo_1.bmp with a 4-bit BMP image file.
  • Remove the old parameter that was commented out from xrdp.ini.in
  • Change default ls_logo_filename to xrdp_logo_1.bmp. (To xrdp_logo_1.png in the imlib2 build)

xrdp/Makefile.am Outdated
@@ -98,6 +98,9 @@ dist_xrdppkgdata_DATA = \
xrdp24b.bmp \
xrdp256.bmp \
xrdp_logo.bmp \
xrdp_logo_1.bmp \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the old logo. The old one can be resurrected from git history if necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger.
I will replace the existing file.

@matt335672
Copy link
Member

Hi @TOMATO-ONE

This all looks fine to me. I haven't tested it yet, as I'm a little busy elsewhere at the moment but I will try to do so later in the week.

@TOMATO-ONE
Copy link
Contributor Author

Re-fixed.

@matt335672
Thanks.

I'm in no hurry.
Please prioritise your work.
I don't believe that the logo redesign needs to be mearge a hurry.

@metalefty
Copy link
Member

It still looks weird.

image

@metalefty
Copy link
Member

Ah, it's my fault.

@metalefty
Copy link
Member

LGTM.

@metalefty metalefty merged commit fafd7bf into neutrinolabs:devel Sep 22, 2022
@TOMATO-ONE
Copy link
Contributor Author

Thank you very much.

I am happy to contribute.

@TOMATO-ONE TOMATO-ONE deleted the new_loginscreen branch September 22, 2022 12:18
@matt335672
Copy link
Member

Thanks @TOMATO-ONE.

Apologies I didn't get to review this earlier in the week.

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.

3 participants