Axes legends is trimmed

[ashokkrish]

Image generated when running a simulation for Crete, Greece.

Can one of you fix this?

[bryce-carson]

Where in the application does this image appear? Which plot, specifically, @ashokkrish?

[bryce-carson]

This is related to #11. These output files are joined to create the MP4 video file; I suspect there is a hard-coded resolution somewhere which leads to the cropping issue, and that the misalignment of the axes labels are related to this or something close by in "context," of course. I'm rambling a little, because obviously the issue wouldn't (or shouldn't) be in unrelated code.

[bryce-carson]

Necessarily, because we want to fix this in stable, however, the solution will be different and independent from the solution for #7 and #11.

[bryce-carson]

spatialEpisim/R/rasterPlot.R

Lines 122 to 124 in 1f6bb3f

	if (!directOutput){
	png(PNGFileName, height = 768, width = 768) # output the plot to the www/ image folder
	}

The size of the image is indeed hard-coded. The resolution of the following image is 768x768.

[ashokkrish]

Thanks for catching this. We will fix it with an appropriate image resolution.

[bryce-carson]

Thanks for catching this. We will fix it with an appropriate image resolution.

It's best to avoid hard-coding the resolution of the image because we aren't targeting a printer and we don't know the size or pixel density of the user's laptop or desktop computer display.

My plan for the unstable branch is in #11. This will be a user-customizable solution, and we can simply feed a bunch of plot objects into whatever hungry function's mouth is necessary to get images prepared for splide (what is splide? I mention that in #11).

For this, I'm unsure what the exact solution will be, but leaving the decision on resolution to the server when it has some information from the user's viewport is likely appropriate; rather than using the user's viewport resolution directly, also, I think the standard web design practice is to just choose the full screen width as the maximum width of the image, then enforce an aspect ratio and leave the height to be determined at run-time. Then the image is automatically scaled down by the user's browser according to the other CSS constraints (see the box model).

I'll do a little research first before pushing a fix to stable.

[bryce-carson]

@kle6951 and @Toby-exe, this issue in the stable branch is a good candidate for the two of you to work on together. Can you review the code related to this (just search MP4) and read it together, then perhaps do some pair programming to resolve it in stable?

[bryce-carson]

@kle6951 and @Toby-exe, pardon me.

Please do not proceed with hacking on this issue.

I have found that some MP4-related code in the software is in the following file.

spatialEpisim/R/rasterSimulation_DA.R

Lines 780 to 790 in 0b3467b

	# MERGE THE PNGs TO A GET AN MP4 VIDEO
	setwd("www/MP4")
	videoDuration <- 15 # in seconds
	av::av_encode_video(list.files(pattern = ".png"),
	framerate = timestep/videoDuration,
	output = paste0(rasterLayer, "_MP4.mp4"))
	setwd("./../..")
	summary[is.na(summary)] <- 0
	write_xlsx(summary, path = paste0("www/MP4/", inputISO, "_summary.xlsx"), col_names = T)

It's not appropriate to work on this file in stable, because it really needs refactoring, so don't bother with the MP4 stuff for now.

@ashokkrish, having discovered this, I think it's inappropriate to proceed with a fix for #23 in stable. I'm unsure why it broken now, if it actually was working before, but I think it may have been broken beforehand without you realizing it. We haven't changed anything that should affect the generation of the plots or the MP4, so really it should've been broken to begin with in stable. Due to the desperate need of a refactor throughout the R/ folder, I don't think it's worth pursuing a fix for #23 in stable, not as the software is now.

[ashokkrish]

As you may have noticed we had hard coded videoDuration <- 15 # in seconds. It was a temporary fix at that for a specific simulation. We could revisit this at some point.

[bryce-carson]

As you may have noticed we had hard coded videoDuration <- 15 # in seconds. It was a temporary fix at that for a specific simulation. We could revisit this at some point.

I have plans in #11 which will be much more robust, and given that won't have the issue of the playback speed or duration that exists in this implementation with a video generated with hard-coded values, it's another argument towards not fixing this issue in stable.

My hope is that the appearance of the plot will simply "fix itself" during the refactor because the erroneous code will be refactored out.