r-lib / gtable

The layout packages that powers ggplot2
https://gtable.r-lib.org
Other
87 stars 18 forks source link

Improve viewport handling and respect the children's viewport. #25

Closed kohske closed 12 years ago

kohske commented 12 years ago

Now we can set a vp of gtable. In addition, gtable respect children's vp, so justification is enable.

Maybe this is not the best way, but easy enough, I think.

Here is the example.

http://rpubs.com/kohske/806

kohske commented 12 years ago

Note that this commit must have bugs, so please do not merge, just in case. I just want to know if this is correct direction.

wch commented 12 years ago

I like the way that this works.

kohske commented 12 years ago

Some modification has been done. You can find the updated example at the bottom of this page: http://rpubs.com/kohske/815

wch commented 12 years ago

This also needs a version bump so we can synchronize it with ggplot2.

wch commented 12 years ago

Here's some code from the second rpub:

gtc <- gtable_matrix("gtc", matrix(gsc, nrow = 2),
                     widths = unit(rep(1, 2), "cm"), heights = unit(rep(1, 2), "cm"))
gtc$vp <- viewport(y = 0, just = "bottom", height = sum(gtc$heights))

Instead of modifying gtc$vp directly, it would be cleaner to add vp as an argument to gtable_matrix (and the other gtable functions).

That said, I'm not 100% sure it makes sense to put a vp flag in the gtable object. What happens to the vp if you wrap the gtable in a gTree?

kohske commented 12 years ago

Instead of modifying gtc$vp directly, it would be cleaner to add vp as an argument to gtable_matrix (and the other gtable functions).

Totally agreed. gtc$vp <- comes from my laziness.

That said, I'm not 100% sure it makes sense to put a vp flag in the gtable object. What happens to the vp if you wrap the gtable in a gTree?

Perhaps the situation is same as the other grobs.

wch commented 12 years ago

That said, I'm not 100% sure it makes sense to put a vp flag in the gtable object. What happens to the vp if you wrap the gtable in a gTree?

Perhaps the situation is same as the other grobs.

Actually, I should have stated that differently. The gtable gets converted to a grob using gtable_gTree(). The returned object is a gTree grob, where the grobs in the gtable become children. So the gtable object isn't part of the gTree -- just the grobs from the gtable. If I understand correctly, the vp from the gtable gets transferred to the vp for the gTree.

However, when you wrap a grob in a gTree, the grob becomes part of the gTree. The vp for the grob does not get transferred to the vp for the gTree.

So, when wrapping in a gTree, the behavior is different between gtables and grobs, right?

kohske commented 12 years ago

Actually, I should have stated that differently. The gtable gets converted to a grob using gtable_gTree(). The returned object is a gTree grob, where the grobs in the gtable become children. So the gtable object isn't part of the gTree -- just the grobs from the gtable. If I understand correctly, the vp from the gtable gets transferred to the vp for the gTree.

ah, yes. gtable can be embedded in another gtable without wrapping gTree. But you need to wrap a gtable by gTree to make it a child of another gTree.

kohske commented 12 years ago

But you need to wrap a gtable by gTree to make it a child of another gTree.

No, no. you don't need to wrap it. You need to convert the gtable into gTree. I will put the example later.

baptiste commented 12 years ago

may I ask again, why is there a need for an intermediate object "gtable"; couldn't gtable be a grob/gTree by itself, with obvious class name "gtable"? As I understand, one can put arbitrary stuff inside a grob.

kohske commented 12 years ago

@wch

here is the example:

grid.newpage()
grid.draw(gtf)
gtf <- gtable_matrix("gtf", matrix(rep(list(rectGrob(gp = gpar(fill = "grey95"))), 4), nrow = 2), 
                     widths = unit(rep(4, 2), "cm"), heights = unit(rep(4, 2), "cm"))

gsc <- lapply(1:4, function(x) rectGrob(width = unit(1, "cm"), height = unit(1, "cm"),
                                        gp = gpar(fill = rainbow(4)[x])))
gtc <- gtable_matrix("gtc", matrix(gsc, nrow = 2),
                     widths = unit(rep(1, 2), "cm"), heights = unit(rep(1, 2), "cm"))
gtc$vp <- viewport(y = 0, just = "bottom", height = sum(gtc$heights))

gtcgt <- gtable_gTree(gtc)

gtc2 <- gtc
gtc2$vp <- viewport(y = 1, just = "top", height = sum(gtc$heights))
gtcgt2 <- gtable_gTree(gtc2)

grid.draw(gTree(children = gList(gtcgt, gtcgt2)))
kohske commented 12 years ago

@baptiste I may agree. It is convenient if gtable is gTree.

hadley commented 12 years ago

I don't think it makes that much for the gtable to be a gTree - the children and viewport of gTree are static, not dynamic (i.e. they are accessed using $ not through some special accessor), so I don't think it would help much.

kohske commented 12 years ago

the children and viewport of gTree are static, not dynamic

But you can manipulate the children and viewport dynamically, and also you can define any accessor. What is difficult is probably updating viewport, though. There is no function like editViewport, while editGrob enables flexible manipulation for a grob.

baptiste commented 12 years ago

@hadley the idea I had was to use the grob as a mere container of arbitrary fields, and get the drawDetails method to set up the gTree to be displayed,

library(grid)

gtableGrob <- function(x, ...){

  g <- gtable_gList(x)
  v <- gtable_viewport(x)

  ## this grob is just a container
  grob(g = g, v = v, cl="gtable", ...)
}

drawDetails.gtable <- function(x){

  ## set up a gTree for drawing
  g <- gTree(children = x$g, childrenvp = x$v, ...)
  grid.draw(g)

}

It may seem silly to have a dummy grob, but I think it's a useful trick to get into grid dispatch mechanism as early as possible without losing the ability to edit the fields.

hadley commented 12 years ago

Ah, ok, that makes sense.

wch commented 12 years ago

This is getting hard to keep track of... Maybe we should just merge this and then start a new branch for implementing Baptiste's idea.

kohske commented 12 years ago

@wch I agree. At the moment this looks working well, so we can write some tests. The new one should be tested against the tests.

baptiste commented 12 years ago

Just so we're clear, I realise that it's much easier to make vague suggestions as a "code tourist" than actually implementing stuff that works :)

occasionally an outsider's view can show things from a user's perspective though, that's why I keep bugging you with such comments

kohske commented 12 years ago

occasionally an outsider's view can show things from a user's perspective though, that's why I keep bugging you with such comments

I know, I know. Actually I'm enjoying. Please feel free to do that, thanks :-)

wch commented 12 years ago

The comments are definitely appreciated. Your knowledge of grid is certainly much better than mine...

wch commented 12 years ago

@kohske Actually, now I think it is better to continue on this branch. But having tests before making big changes is definitely a good idea.

kohske commented 12 years ago

moved to #28 so I will close this issue.