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

[BUG] Recent update causes angled axis labels to be mis-positioned #3776

Closed
BioTurboNick opened this issue Aug 27, 2021 · 13 comments · Fixed by #3782
Closed

[BUG] Recent update causes angled axis labels to be mis-positioned #3776

BioTurboNick opened this issue Aug 27, 2021 · 13 comments · Fixed by #3782
Assignees
Labels

Comments

@BioTurboNick
Copy link
Member

BioTurboNick commented Aug 27, 2021

Details

image

It looks like the tick labels are being rotated around their center, but not offset like they used to be.

Backends

This bug occurs on ( insert x below )

Backend yes no untested
gr (default) x
pyplot x
plotly x*
plotlyjs x
pgfplotsx x
inspectdr x
  • plotly crashes in Plots.tick_padding with ERROR: MethodError: no method matching length(::Date) within a call to maximum

Versions

Plots.jl version: 1.21.1
Backend version (]st -m <backend(s)>): 0.58.1
Output of versioninfo():

Julia Version 1.6.2
Commit 1b93d53fc4 (2021-07-14 15:36 UTC)
Platform Info:
  OS: Windows (x86_64-w64-mingw32)
  CPU: Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, skylake)
Environment:
  JULIA_EDITOR = code
  JULIA_NUM_THREADS =
@t-bltg
Copy link
Member

t-bltg commented Aug 27, 2021

Probably caused by #3587.

There is a trigger on alignment for 45°, can you try with a greater rotation angle e.g. 46° ?

@BioTurboNick
Copy link
Member Author

45 and higher seems to be fine. Lower than 45 has the problem.

@t-bltg t-bltg self-assigned this Aug 28, 2021
@t-bltg
Copy link
Member

t-bltg commented Aug 28, 2021

I still have to think what we should do here. Goal is to keep #3581 fixed.

We could hardcode a factor e.g.

function gr_set_tickfont(sp, letter)
    ...
    # invalidate alignment changes for small rotations (|θ| < θᵣ)
    hardcoded_factor = 2
    trigger(rot) = hardcoded_factor * abs(sind(rot)) < abs(cosd(rot)) ? 0 : sign(rot)
    rot = axis[:rotation]
    if letter === :x || (RecipesPipeline.is3d(sp) && letter === :y)
        halign = (:left, :hcenter, :right)[trigger(rot) + 2]
        valign = (axis[:mirror] ? :bottom : :top, :vcenter)[trigger(abs(rot)) + 1]
    else
        ...
    end
    ...
end

, but I prefer to avoid piracy and propose something generic. @BioTurboNick, maybe you have a suggestion for this ?

mwe

using Measures, Random, Dates, Plots
Random.seed!(2018)

main() = begin
  days = 31
  position = cumsum(randn(days))
  x = Date(2018, 1, 1):Day(1):Date(2018, 1, 31)
  ticks = [x[i] for i in 1:5:length(x)]

  for θ  0:10:120
    plot(
      x, position,
      xlabel = "Date",
      ylabel = "Position",
      title = "Track of random walker",
      xticks = ticks,
      xrotation = θ
    )
    png("bug-")
  end
  return
end

main()

@BioTurboNick
Copy link
Member Author

I'll give it some thought.

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Aug 29, 2021

What if any angle other than 0, 90, 180, 270 positions the corner at the tick?
image

@t-bltg
Copy link
Member

t-bltg commented Aug 29, 2021

Isn't that what's currently done ?

I was thinking of maybe increasing offset based on rotation:

y_offset = isy ? 0 : -8e-3 * out_factor

@t-bltg t-bltg added the GR label Aug 29, 2021
@BioTurboNick
Copy link
Member Author

What's currently being done is switching between centering the tick on the middle of the top, to centering the tick on the middle of the side. Like so:
image

@t-bltg
Copy link
Member

t-bltg commented Aug 29, 2021

Ha yeah, wrote this PR 2 months ago and already forgot what I coded 😆.

Some possible fix:

# invalidate alignment changes for small rotations around default position
trigger(rot) = abs(sind(rot)) < .25 ? 0 : sign(rot)

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Aug 29, 2021

There's probably a simpler way to write this but this is a little more general for all rotations I think:

    rot = axis[:rotation] % 360
    if letter === :x || (RecipesPipeline.is3d(sp) && letter === :y)
        if -90 < rot < 90 || rot < -270 || rot > 270
            valign = axis[:mirror] ? :bottom : :top
        elseif abs(rot) % 90 == 0 # -270, -90, 90, 270
            valign = :vcenter
        else
            valign = axis[:mirror] ? :top : :bottom
        end

        if 0 < rot < 360
            halign = axis[:mirror] ? :left : :right
        elseif abs(rot) % 180 == 0 # -180, 0, 180
            halign = :hcenter
        else
            halign = axis[:mirror] ? :right : :left
        end
    else
        if 0 < rot < 360
            valign = axis[:mirror] ? :top : :bottom
        elseif abs(rot) % 180 == 0 # -180, 0, 180
            valign = :vcenter
        else
            valign = axis[:mirror] ? :bottom : :top
        end

        if -90 < rot < 90 || rot < -270 || rot > 270
            halign = axis[:mirror] ? :left : :right
        elseif abs(rot) % 90 == 0 # -270, -90, 90, 270
            halign = :hcenter
        else
            halign = axis[:mirror] ? :right : :left
        end
    end

@BioTurboNick
Copy link
Member Author

BioTurboNick commented Aug 29, 2021

Suggest this modification to look at y axis and mirrored axes simultaneously.

main() = begin
  days = 31
  position = cumsum(randn(days)) .* 10000
  x = Date(2018, 1, 1):Day(1):Date(2018, 1, 31)
  ticks = [x[i] for i in 1:5:length(x)]

  for mir  [true, false]
    for θ  -120:10:120
      plot(
        x, position,
        xlabel = "Date",
        ylabel = "Position",
        title = "Track of random walker",
        xticks = ticks,
        xrotation = θ,
        yrotation = θ,
        xmirror = mir,
        ymirror = mir
      )
      png("bug--$mir")
    end
  end
  return
end

@t-bltg
Copy link
Member

t-bltg commented Aug 29, 2021

There's probably a simpler way to write this but this is a little more general for all rotations I think:

Looks good, explicit is sometimes more readable. Can you make a PR out of it ? You'll need to regenerate reference images for GR I guess.

Suggested changes writing if clauses:

valign = if -90 < rot < 90 || rot < -270 || rot > 270
    axis[:mirror] ? :bottom : :top
elseif abs(rot) % 90 == 0 # -270, -90, 90, 270
    :vcenter
else
    axis[:mirror] ? :top : :bottom
end

[...]

@BioTurboNick
Copy link
Member Author

I'm not sure how the reference images work, where do I start with that?

@t-bltg
Copy link
Member

t-bltg commented Aug 29, 2021

I'm not sure how the reference images work, where do I start with that?

Ref: https://docs.juliaplots.org/latest/contributing/#VisualRegressionTests

  1. Install PlotReferenceImages: https://github.com/JuliaPlots/PlotReferenceImages.jl#installation.
  2. Test Plots: https://github.com/JuliaPlots/PlotReferenceImages.jl#usage (with your patch).
  3. Then submit a PR to PlotReferenceImages.jl with the modified reference images, complementing the one made in Plots.jl.

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

Successfully merging a pull request may close this issue.

2 participants