-
Notifications
You must be signed in to change notification settings - Fork 160
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
24.06 - "Allgemein" is cut off if HTML is selected -> scaling @200% #890
Comments
Since I already have some expericence because I do translation for MPC-BE, MSI Afterburner, RTSS, Icaros and XnViewMP, I can join help german translation work if you want. |
@cjee21 could you check? |
If I look closer some more entries are cut off below |
Thanks for reporting, partially reproducible after the new WebView2 in-progress PR when falling back to IE engine. I'll do more tests and improve it later.
I think the developers will like that. |
development snapshot, @Klaus1189 please test. |
This doesn't include the MediaInfoLib PR 2084? |
Oops, I relaunch. |
Need the HTML injections from MediaInfo PR 873 too. |
FYI my screenshots are taken with all my PRs (including non-HTML-related ones) from MediaInfo and MediaInfoLib merged. I tested to ensure all PRs work well together. |
873 can not yet be merged/tested as is, please extract all not related to EdgeView in 892. |
They are all related except the first one that shouldn't cause issues. The last one depends on if 64-bit is merged first or not. I can remove first one since that is now in 892. Update: @JeromeMartinez done |
dev snapshot with up to date MediaArea/MediaInfoLib#2084 and #892. |
Oh wait, you are using 892 for this? I thought using 873 so I made that build-able and merge-able... If you meant move the HTML injections for IE to 892, the code for both engines are in the same function in the same commit so it's difficult to split... Just tested, confirmed HTML view is even more broken in that snapshot since 892 does not contain any HTML changes. |
I won't merge any change to .bat, .nsi, WebView2 related stuff until I test them more deeply.
I don't see why a 2nd engine is mandatory for the HTML changes, the code impacting the display should be independent of a 2nd engine integration. If it is not independent it is another reason for me to not merge 873., too complicated.
It seems better to me but I don't have 200% DPI so I can not test all. Really, the 2nd web engine should be a totally separate topic, as well as 64-bit stuff, I am fully aware that the current code is ugly but replacing it by something inter-dependent would be even more difficult later. There should be a way to have all HTML impacting code separate from the count of HTML engines, all HTML only code which does impact the old engine should be able to be there without the new engine presence. |
It is because I re-wrote the function to inject HTML styles and used the same function for both engines. Then I only made one commit with all changes. If you want split commits, I can do it but additional work.
No borders and inconsistent column widths here on 100%. So to be clear, do you want to split all code necessary for handling MediaInfoLib 2084 + IE Engine to a separate commit and put in 892, then change 873 to be built on-top of 892? |
More or less that. If too much work, we freeze the current work on HTML fixes until I review 873. |
I will see how much work it requires. Looking at the cpp now. |
@JeromeMartinez done putting it in 892. This part is not that difficult. More difficult will be cleaning up 873 later I think. So you will prefer if I split 873 into smaller easier to understand commits? |
This one does not work, same issue.
This one works, but a small issue: the bottom lines (Bild) are more to the right, or the top lines (Allgemein) are more to the left. I think it would be best to be in one line |
I must also add that 200% scaling looks so much fresher than older versions. Some things to tweak and it looks perfect |
@Klaus1189 please also confirm if it satisfactorily resolves your issue 889. |
Should be small enough after rebase. |
It's a pass/OK for me. |
Now it is fine for me, Allgemein is not cut off, so this issue 890 is solved. But it is now fully black, before it was not: 889 not fully solved, still a bit off: |
The more I think about, I think the grey is more pleasing for the eyes. What do you think? |
889 is not fully solved |
@cjee21 I think I also prefer the old greyish color, could you put it back in another PR. |
Good.
Intended. It is not fully black but now follow Edge browser's dark theme colour because planning to implement Edge WebView2 in the future and WebView2 has native dark theme which does not require manually selecting the background/font colours.
I don't think it is possible to get exact match with the current state of the codes. It is perfect on 100% DPI. |
Bummer. |
Isn't it possible to force grey background for the older HTML engine? |
Something I can fix, I think, looks like just positioning issue, @cjee21 if you like to play to try I let you :-p else I'll check that later. |
If you want the inconsistency between the two engines, just change this line: MediaInfo/Source/GUI/VCL/GUI_Main.cpp Line 164 in dda61d6
You can change |
See it's perfect here. That is why it is hard to fix. Need to test light, dark, different DPI. I'm not going to do this. One or 2 pixels off there is not important in my opinion and how often does that window get used anyway. |
I am not sure to understand. My understanding is that we need to have a fix color for the older engine and that for the new engine it will be from the selected theme, isn't it?
What was the older value? |
The new engine uses #121212 as background when in dark theme (this has nothing to do with VCL themes and is the same as Edge browser when you switch Windows to dark scheme). So I used that for old engine too for consistency, so that whatever engine is used, the display is as similar (other than Edge handling DPI scaling of column widths better) as possible.
Here: MediaInfo/Source/GUI/VCL/GUI_Main.cpp Line 150 in d5e8e94
Furthermore, is has been more obviously off for years isn't it? |
@JeromeMartinez The old value is matching the text view background. Side note: We might get complains from OLED screen users next about how the dark theme is not close to completely black and causing battery drain and screen aging 😆 |
An option! :-p |
What about using it for both engines? |
Export HTML from the new version, then open the HTML in Edge, then change Windows to dark mode. That's the default dark background. You can see there are no colour codes in the HTML source code. |
@JeromeMartinez about the Qt version: This is how it looks after recent HTML changes:
If it is desired to have proper HTML rendering and identical display as VCL version.... The good news is I have a 4-line patch to achieve the results below:
The not so good news is that with this patch, Qt will bundle an entire web engine with MediaInfo which greatly increases total size. Therefore, the patch remains on a branch in my fork without a PR. Note that Qt's dark mode HTML background is also 121212. |
"Allgemein" is cut off if HTML is selected -> scaling @200%
See in this screenshot:
The text was updated successfully, but these errors were encountered: