JBotSim icon indicating copy to clipboard operation
JBotSim copied to clipboard

BackgroundPainter called in unexpected order

Open remikey opened this issue 6 years ago • 2 comments

When using several BackgroundPainters (several call to Topology.addBackgroundPainter()), we would expect that the last added be called last (at it is the case when handling LinkPainter and NodePainter). It is not the case.

Please correct this.

remikey avatar Oct 15 '19 08:10 remikey

As we discussed this morning, for now, one (easy) option is to insert background painters in penultimate position when Topology.addBackgroundPainter()) is called (the last position being used by an inner painter that draws the sensing ranges). If you feel comfortable with this, then it is fine with me. Otherwise, we can discuss further.

acasteigts avatar Oct 15 '19 13:10 acasteigts

Although I first liked the idea of this elegant quick fix, after thinking about it for some time yesterday, I can see painful drawbacks to this approach:

  • Since the user is allowed to remove/replace the "inner painter" (using Topology.setDefaultBackgroundPainter()), it would require that we make sure that the last painter is our painter. Both of the following cases are relevant and would complicate understanding of the painters mechanism:

    • Using JBackgroundPainter.class: Any implementation trying to enhance the JBackgroundPainter capabilities by inheriting from it (as it is done with the JBackgroundPainterHD by adding anti-aliasing) would not be considered as the "inner painter" and thus, not displayed on top.
    • Using instanceof JBackgroundPainter: Any lazy implementation done by inheriting from JBackgroundPainter, instead of implementing BackgroundPainter, would be considered as the original inner painter and thus displayed on top, although it is expected to be last.
  • It would make the code harder to read (and harder the maintenance).

Discussion

I think that the issue is that we use a BackgroundPainter to draw something that we actually want on top of anything on the background (the LinkPainter layer has naturally been placed in the right position). The current content of our JBackgroundPainter makes it more a JSensingRangePainter.

This could be solved by adding a layer drawn after the BackgroundPainter layer (and before or after the LinkPainter layer). Possible alternatives (any of which would make things clearer):

  1. I am tempted to add something like an IntermediatePainter that would fix this problem and future ones, but I suppose that we might end up with the same kind of problem with the default sensing range being drawn after/before something else (?).

  2. A dedicated layer for a SensingPainter interface can also be created. This would fix this specific problem.

  3. The combination of both would make sure this is fixed and might prevent future issues.

remikey avatar Oct 16 '19 08:10 remikey