Skip to content
This repository has been archived by the owner on Jul 8, 2022. It is now read-only.

improve debugger #442

Merged
merged 12 commits into from
Mar 13, 2022
Merged

improve debugger #442

merged 12 commits into from
Mar 13, 2022

Conversation

bognari
Copy link
Contributor

@bognari bognari commented Feb 18, 2022

This will hopefully help to make the debug font more readable on height resolutions with small displays.

partial fix / workaround for korlibs/korge#556

…is will hopefully help to make the debug font more readable on height resolutions with small displays.

fun addDebugRenderer(block: Views.(RenderContext) -> Unit) {
fun addDebugRenderer(block: Views.(RenderContext, Double) -> Unit) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing it as a primitive as an extra argument, can we put this extraFontScale inside the RenderContext? Either as a property or as an extrinsic Extra.Property? Maybe with a name like debugExtraFontScale or something?

Additionally, are you doing this because we are not properly detecting the device pixel ratio on windows? Could we implement that instead?

@bognari
Copy link
Contributor Author

bognari commented Feb 21, 2022

Hey @soywiz I did you requestet change and insertet the debugExtraFontScale into the RendererContext.

Maybe you are right and it would be a better idea to implement the device pixel ratio correctly but maybe for someone the font is still to small so it is possible to make the font bigger (or smaller).

@bognari
Copy link
Contributor Author

bognari commented Feb 21, 2022

ok now we use the scaling settings too.
https://stackoverflow.com/questions/32586883/windows-scaling

@soywiz
Copy link
Contributor

soywiz commented Feb 22, 2022

Awesome! Thanks :) let's wait for CI to finish

// somehow this value is not update when you change the scaling in the windows settings while the jvm is running :(
val pixelsPerLogicalInchRatio = Toolkit.getDefaultToolkit().screenResolution / defaultPixelsPerLogicalInch

return@lazy (scale * pixelsPerLogicalInchRatio)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tested on mac. On mac, it should be something like scale / pixelsPerLogicalInchRatio.

But I believe we should separate those values:

override val devicePixelRatio: Double get() = ... scaleX
override val pixelsPerLogicalInchRatio: Double get() = ... screenResolution / defaultPixelsPerLogicalInch
// Use this in the debug handler, while allowing people to access raw devicePixelRatio without the noise of window scaling
override val computedPixelRatio: Double get() = devicePixelRatio / pixelsPerLogicalInchRatio

Otherwise devicePixelRatio stops being true. Is it 1 on your hdpi screen? On mac devices with retina displays it is 2, without it, it is 1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "scale" value on my windows laptop is always 1 so the multiplication was just an assumption

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But yes we can separate the values.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm still not sure about the calculation of "computedPixelRatio" becasue in my opinion it must be

devicePixelRatio * pixelsPerLogicalInchRatio
or
pixelsPerLogicalInchRatio / devicePixelRatio

@bognari bognari changed the title Add possibility to set an extra font scale for the debug handlers. improve debungger Mar 9, 2022
@bognari bognari changed the title improve debungger improve debugger Mar 9, 2022
@bognari
Copy link
Contributor Author

bognari commented Mar 9, 2022

@soywiz any feedback about my pr?

@soywiz
Copy link
Contributor

soywiz commented Mar 9, 2022

Yes, sorry for the delay. Had a pretty long couple of weeks, and forgot to check this. Im not at home right now, but Ill check it asap

@bognari
Copy link
Contributor Author

bognari commented Mar 9, 2022

sorry. all is good. take your time. this was just a reminder

@@ -46,6 +46,8 @@ class RenderContext constructor(
val views: Views? = bp as? Views?

var debugAnnotateView: View? = null
var debugExtraFontScale : Double = 1.0
var debugExtraFontColor : RGBA = Colors.WHITE
Copy link
Contributor

Choose a reason for hiding this comment

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

Downloaded the PR, and this line seems to fail because RGBA is not found.
You should add a import com.soywiz.korim.color.*

Copy link
Contributor

@soywiz soywiz left a comment

Choose a reason for hiding this comment

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

Sorry for the delay.

I have tried it and seems to work properly, though you should add a import com.soywiz.korim.color.* for it to compile.

Once you add that import, let's merge this ^^

@soywiz soywiz merged commit 377a559 into korlibs-archive:main Mar 13, 2022
@soywiz
Copy link
Contributor

soywiz commented Mar 13, 2022

Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants