Intent of NavigatableComponent$isVisibleOnScreen()

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Intent of NavigatableComponent$isVisibleOnScreen()

Robert Scott
Hi,

Just trying to decode some of the reasoning behind bits of
NavigatableComponent - was wondering what case exactly NavigatableComponent
$isVisibleOnScreen() is intended to protect updateLocationState() from.

I'm looking at it thinking whether it's possible to make it more granular with
a view to being able to use MapView more fully in tests, and its presence
confuses me - if it's possible to patch it out with just a constant true (e.g.
in gui.mappaint.RenderingHelper, even in headless mode, what is its purpose?

Is it an optimization? Is it for cases when it might get called before a
NavigatableComponent's added to a visible window? Is it for the CLI mode?

If I knew I'd be able to see whether it can be replaced with something more
specific.


robert.


Reply | Threaded
Open this post in threaded view
|

Re: Intent of NavigatableComponent$isVisibleOnScreen()

Paul Hartmann-2
On 20.11.2017 00:09, Robert Scott wrote:

> Hi,
>
> Just trying to decode some of the reasoning behind bits of
> NavigatableComponent - was wondering what case exactly NavigatableComponent
> $isVisibleOnScreen() is intended to protect updateLocationState() from.
>
> I'm looking at it thinking whether it's possible to make it more granular with
> a view to being able to use MapView more fully in tests, and its presence
> confuses me - if it's possible to patch it out with just a constant true (e.g.
> in gui.mappaint.RenderingHelper, even in headless mode, what is its purpose?
>
> Is it an optimization? Is it for cases when it might get called before a
> NavigatableComponent's added to a visible window? Is it for the CLI mode?
>
> If I knew I'd be able to see whether it can be replaced with something more
> specific.

This code is by Michael Zangl, so he would know much more about it. From
my understanding, isVisibleOnScreen() prevents the recalculation of the
location state when the component has not been added to the visible window.
At the same time, the method is intended to be overridden with constant
true in a situation where the NavigatableComponent is handled completely
off-screen (RenderingHelper and unit tests, see also [10405]).

It would be good if RenderingHelper could be compiled without any
dependency on Swing classes (i.e. JComponent as super class of
NavigatableComponent). Michael has achieved a lot in this direction, but
there is still more to be done. :)

Paul

Reply | Threaded
Open this post in threaded view
|

Re: Intent of NavigatableComponent$isVisibleOnScreen()

Robert Scott
On Monday, 20 November 2017 22:45:04 GMT Paul Hartmann wrote:

> ...
> This code is by Michael Zangl, so he would know much more about it. From
> my understanding, isVisibleOnScreen() prevents the recalculation of the
> location state when the component has not been added to the visible window.
> At the same time, the method is intended to be overridden with constant
> true in a situation where the NavigatableComponent is handled completely
> off-screen (RenderingHelper and unit tests, see also [10405]).
>
> It would be good if RenderingHelper could be compiled without any
> dependency on Swing classes (i.e. JComponent as super class of
> NavigatableComponent). Michael has achieved a lot in this direction, but
> there is still more to be done. :)

Thanks for this. I have actually just been trying the tests with it patched
out to true and indeed it doesn't seem to cause any additional failures or
errors on my system.

I'm thinking it would be extremely useful for the tests if the standard class
were able to have a flag set (and unset) which would hard-wire this to true.
This way, any of the complicated mocking or JOSMFixture re-initialization
that might be required in other solutions I've been investigating would be
necessary.

Hell, it might be as simple as adding a straightforward
GraphicsEnvironment.isHeadless() || (...) before the existing expression.


robert.