User talk:VladVukicevic

Discussion with bz re canvas:

 21:51 -!- bz [bzbarsky@ppp-69-217-200-129.dsl.chcgil.ameritech.net] has joined #canvas
 21:52 -!- shaver [shaver@moz-C4D14A2E.net] has joined #canvas
 21:52 < bz> We should really wiki this up....
 21:52 <@vlad> which bit?
 21:52 < bz> Maybe we can copy-paste into there
 21:52 < shaver> yeah, IRC first
 21:52 < bz> Whatever I type now.  ;)
 21:52 < shaver> then we dump on a wiki page
 21:52 <@vlad> oh, yeah
 21:52 < shaver> then we point the devmo-horde at it
 21:52 <@vlad> I'll wiki it up
 21:52 < bz> awesome
 21:52 < bz> So the basic idea is that we have this concept of "change hints"
 21:53 < bz> Which are hints that indicate what's actually changed
 21:53 < bz> And what course of action should be taken, therefore
 21:53 < bz> typical hints would be "reflow"
 21:53 <@vlad> this is the hint returned by GetAttributeChangeHint() ?
 21:53 < bz> "repaint"
 21:53 < bz> "reconstruct frame"
 21:54 < bz> These hints can come from several different places
 21:54 < bz> The total hint is a bitfield
 21:54 < bz> We construct the total hint for a change, then look at what bits are set and do the right things
 21:54 < bz> eg reflow _and_ repaint
 21:54 < bz> or whatever needs to happen
 21:55 < bz> So that's the first idea
 21:55 <@vlad> ok
 21:56 < bz> The second idea is that some of our attributes are "mapped into style"
 21:56 <@vlad> so for canvas, I can now make that do just reflow instead of framechange now, now that the frame does Reflow w/h correctly
 21:56 < bz> What this means is that these attributes somehow directly map to things in CSS
 21:56 < bz> For example, the width attribute on images
 21:57 < bz> For various reasons the way we handle that is by actually constructing a style rule
 21:57 < bz> That, instead of being a CSS style rule, is an attribute style rule.
 21:57 < bz> Then when we're resolving style we just ask that rule to map in its style data.
 21:57 < bz> This is what it means for an attribute to be "mapped into style"
 21:57 < bz> So the way the total hint for an attribute change is computed is the following:
 21:58 < bz> 1)  Ask the content node what the explicit hint for the attribute is.  This is GetAttributeChangeHint().
 21:59 < bz> 2)  Check whether the node's style depends on this attribute.  This looks at attribute selectors in CSS and looks at the results of the IsAttributeMapped() method
 22:00 < bz> If the style depends on the attribute, we compute a new style context, and compute a difference between the old and new style context.
 22:00 < bz> This difference is a change hint, and we OR it with the result of step 1.
 22:00 <@vlad> okay, so question
 22:00 <@vlad> and i'm not sure how to phrase this
 22:01 < bz> 3)  If there is native theming involved, we ask the native them impl whether the attr change is something we need to repaint for.
 22:01 < bz> If it is, we toss a "repaint" hint into the bitfield
 22:01 < bz> And that's our total change hint.
 22:01 < bz> vlad: ok, you have the floor.
 22:01 < shaver> the chair recognizes vladimir
 22:02 <@vlad> for #2
 22:02 <@vlad> if IsAttributeMapped is true for a given attribute
 22:02 <@vlad> then that node's style always depends on that attribute?
 22:02 <@vlad> and a change hint gets computed every time an attribute is changed?
 22:03 < bz> If IsAttributeMapped is true for a given attribute, that will cause us to recompute style when the attribute changes, yes
 22:03 <@vlad> ok
 22:04 < bz> For the every time bit, "almost"
 22:04 <@vlad> so I don't really need GetAttributeChangeHint in my canvas element then?
 22:04 < bz> I don't know 
 22:04 < bz> Depends on what attributes you have that can change
 22:04 < bz> And how those would affect your element
 22:04 <@vlad> well, for canvas
 22:04 <@vlad> it's only width/height
 22:05 < bz> So the only attributes for that tag that affect its rendering are width and height?
 22:05 <@vlad> (IMO, they shouldn't be mapped into CSS, but we sort of have to to avoid confusion, and it's what the spec says.. though I should check safari)
 22:05 <@vlad> yes
 22:05 < bz> (and whatever general HTML attrs)
 22:05 < bz> Yeah
 22:05 < bz> Then you don't need GetAttributeChangeHint if they map into CSS
 22:05 <@vlad> and when those change I need a reflow to happen
 22:05 < bz> Yeah
 22:06 < bz> Then the way to do that is to have IsAttributeMapped return true for those
 22:06 < bz> And to map them into style
 22:06 < bz> The style context code knows that if a computed width or height is different we need to reflow
 22:06 < bz> See nsStylePosition::CalcDifference
 22:07 < bz> http://lxr.mozilla.org/seamonkey/sou...Struct.cpp#943
 22:07 <@vlad> related question: nsGenericHTMLElement::sImageMarginSizeAttributeMap contains width, height, hspace, vspace
 22:07 < bz> yes
 22:07 <@vlad> should that be split into two sets, one with just width/height, and one with hspace/vspace?
 22:07 < bz> Which is a little odd
 22:08 < bz> Probably not a bad idea
 22:08 < bz> Then you could use one and not the other
 22:08 <@vlad> yeah
 22:08 < bz> Check other places where ::height is used?
 22:08 <@vlad> what shold the hspace/vspace one be called?
 22:08 < bz> I bet other places could use this too
 22:08 < bz> I'd call that sImageMarginSizeAttributeMap
 22:08 < bz> And call the other sImageSizeAttributeMap
 22:08 <@vlad> k
 22:09 < bz> Or maybe sImageMarginAttributeMap and sImageSizeAttributeMap
 22:09 < bz> It's weird, since we have a single map but separate methods to map into style
 22:09 <@vlad> or sSizeAttributeMap
 22:09 < bz> Yeah
 22:09 < bz> Or that
 22:09 <@vlad> because ImageMarginSizeAttributeMap is used by applet, image, input, object
 22:09 < bz> Hmm
 22:09 < bz> And not all of those map in vspace/hspace, I bet?
 22:10 < bz> input and image have to
 22:10 <@vlad> applet seems to
 22:10 <@vlad> http://lxr.mozilla.org/seamonkey/sou...lement.cpp#182
 22:10 <@vlad> and <object>
 22:10 < bz> heh
 22:10 < bz> ok, then
 22:10 < bz> Of course none of this is in the HTML spec
 22:10 <@vlad> heh
 22:10 < bz> But that's because vspace/hspace aren't in the spec at all
 22:11 < bz> Any more.
 22:11 <@vlad> ah
 22:11 < bz> Used to be in 3.2, iirc
 22:11 < shaver> it's in the HTML spec that is hosted by web.archive.org and google.com
 22:11 <@vlad> exciting.
 22:11 < bz> actually, wait
 22:11 < bz> hspace and vspace are in 4.0 but deprecated
 22:11 <@vlad> so i'm going to go for sSizeAttributeMap and sMarginAttributeMap
 22:11 < bz> apply to img/applet/object
 22:11 < bz> we want to apply them to image inputs too
 22:12 <@vlad> ok
 22:12 < bz> Sounds good to me
 22:12 <@vlad> well, so
 22:12 <@vlad> there is a group of people who think <canvas> should be an <img>
 22:12 < shaver> are you changing nsGenericHTMLElement
 22:12 < shaver> ?
 22:12 <@vlad> so maybe we can just have <canvas> handle hspace and vspace and be done with it
 22:12 < shaver> vlad: that group is wrong, IMO
 22:12 < shaver> but you've heard that before!
 22:12 <@vlad> shaver: sure, but in this instance?
 22:13 <@vlad> hspace/vspace don't seem to hurt anything
 22:13 < shaver> oh
 22:13 < shaver> correct
 22:13 < bz> That would be fine too
 22:13 < shaver> it should resemble them in this way
 22:13 <@vlad> yeah
 22:13 < bz> my review comment was that we claimed our style depended on them
 22:13 < bz> But it actually didn't
 22:13 < bz> As long as we're consistent, I'm happy.  ;)
 22:13 <@vlad> ok, so
 22:13 <@vlad> let's talk about style for a second
 22:14 < bz> ok
 22:14 <@vlad> you say we claimed our style depended on them, but actually it didn't
 22:14 <@vlad> where did the not-depending come into play?
 22:14 <@vlad> in the frame?
 22:15 < bz> bug#?
 22:15  * bz wants to look at the patch to make sure he gets this right... ;)
 22:15 <@vlad> also, and possibly related, right now the canvas element isn't really style-able.. like border: solid 5px blue; doesn't do anything
 22:15 <@vlad> https://bugzilla.mozilla.org/show_bug.cgi?id=291216
 22:16 < bz> here we are
 22:16 < bz> So we had
 22:16 < bz>  nsHTMLCanvasElement::IsAttributeMapped
 22:16 < bz>      sCommonAttributeMap,
 22:16 < bz>       sImageMarginSizeAttributeMap
 22:17 < bz> So that claims that our style depends on the attributes listed in those maps
 22:17 < bz> Whenever one of those changes, our style will be reresolved
 22:17 <@vlad> ok
 22:17 <@vlad> and taht style reresolve is supposed to affect the frame?
 22:18 <@vlad> but my HTMLCanvasFrame is too simple?
 22:18 < bz> hold on
 22:18 < bz> The style reresolve takes all style rules applying to the element
 22:18 < bz> And computes the new style
 22:18 < bz> Note that the set of style rules is a superset of the set of CSS rules
 22:19 < bz> There's also the "attribute rule"
 22:19 < bz> Which maps in style from attributes
 22:19 < bz> This is done, for HTML, by nsGenericHTMLElement::WalkContentStyleRules
 22:20 < bz> Which, when all is said and done, is calling nsHTMLCanvasElement::GetAttributeMappingFunction
 22:20 < bz> In our case
 22:20 < bz> And then calling whatever that returned
 22:20 < bz> So it's calling that static MapAttributesIntoRule we defined
 22:21 < bz> Which does the actual work of mapping in the data
 22:21 < bz> Now our MapAttributesIntoRule was done as:
 22:21 < bz> +  nsGenericHTMLElement::MapImageSizeAttributesInto(aAttributes, aData);
 22:21 < bz>    nsGenericHTMLElement::MapCommonAttributesInto(aAttributes, aData);
 22:21 < bz> Which maps in width and height
 22:21 <@vlad> ah
 22:21 < bz> and some stuff like dir and whatnot
 22:21 <@vlad> so i'm never mapping in hspace/vspace?
 22:21 < bz> But not vspace or hspace
 22:21 < bz> Right
 22:22 < bz> MapImageMarginAttributeInto
 22:22 <@vlad> yeah
 22:22 < bz> is what you probably want
 22:22 <@vlad> (whee, good thing there's 2 separate function calls there, but only one list!)
 22:22 < bz> heh
 22:22 < bz> Yeah, that's a little silly... ;)
 22:23 < bz> So yeah, none of that had anything to do with the frame
 22:23 <@vlad> okay
 22:23 <@vlad> so assuming that stuff was fixed
 22:23 <@vlad> as it just was, I think, in my tree
 22:23 <@vlad> I should be able to style="width: 100px; height: 100px" ?
 22:24 < bz> And that would override the width/height attrs on the node
 22:24 < bz> yes
 22:24 < bz> Note that there is then the issue of what you compare to mImageFrame
 22:25 <@vlad> in UpdateImageContainer?
 22:25 < bz> Yeah
 22:25 < bz> See, the problem is that you can have the same HTMLCanvasElement rendering at two different sizes
 22:25 < bz> if you allow CSS to apply
 22:26 < bz> One size in screen media, a different size in print media, for example
 22:26 <@vlad> oh.
 22:26 <@vlad> so roc had a suggestion that I thought was good, but is confusing
 22:26 <@vlad> and it's in the spec right now, though apple doesn't do it
 22:26 <@vlad> which is, width/height on <canvas> are the size of the rendernig context
 22:26 <@vlad> but you can scale it using CSS width/height
 22:27 <@vlad> but I think that <canvas width="100" height="100" style="width: 200px; height: 200px;"> is pretty confusing
 22:27 < shaver> min/max/pref, baby!
 22:27 <@vlad> but also apple allows width="50%"
 22:27 <@vlad> I have no idea how to implement percents, to be honest
 22:28 < bz> one sec.  phone.
 22:28 <@vlad> np
 22:31 < bz> So
 22:32 < bz> So the suggestion is that width/height on the canvas are its intrinsic size
 22:32 < bz> And then its computed size is just done like any replaced element
 22:32 < bz> using CSS if it's there, and intrinsic size otherwise?
 22:35 < bz> If we map the width/height into CSS, percents are easy
 22:35 < bz> Otherwise, it's not quite clear what they should mean...
 22:37 <@vlad> yeah
 22:37 <@vlad> I'm not really sure what they mean for apple, either
 22:37 <@vlad> well
 22:37 <@vlad> it's clear what they mean on a page
 22:38 < bz> Hmm?
 22:38 < bz> You mean just percent of the page?
 22:38 <@vlad> no, I mean, just like img width/height of %
 22:39 < bz> Right
 22:39 < bz> That's if it's mapped into style
 22:39 <@vlad> but the problem is that the underlying canvas has to be a pixel size
 22:39 <@vlad> so if it gets resized for, say, printing
 22:39 < bz> Then it's not clear what they mean
 22:39 <@vlad> then the canvas gets cleared
 22:39 <@vlad> which is not what they intend
 22:40 < bz> right
 22:40 < shaver> hmm
 22:40 <@vlad> so if wedidn't support %
 22:40 < shaver> do we have that onResize handler?
 22:40 <@vlad> no
 22:41 <@vlad> if we didn't support %, I'd have to write my own MapImageSizeAttributesInto
 22:41 <@vlad> not a big deal
 22:41 < bz> yeah
 22:41 < bz> Well, wait
 22:41 < shaver> I think not supporting % is a good plan so far
 22:41 < bz> wait.
 22:42 < bz> So the fundamental question is what width/height mean
 22:42 < bz> Are they the size of the canvas to be painted on?
 22:42 < bz> Or the size the canvas should be rendered at?
 22:42 <@vlad> at some level they have to become the size of the canvas to be painted at
 22:42 < bz> If the latter, where does the former come from?
 22:42 <@vlad> in pixels
 22:42 < bz> ok
 22:42 <@vlad> not necessarily the width/height attributes, though safari already shipped, and they use w/h
 22:42 < bz> Right
 22:43 < bz> So the simple way to do this
 22:43 < bz> is to set them to be the width/height of the painting surface
 22:43 < bz> And _not_ map them into CSS
 22:43 <@vlad> and do the scaling?
 22:43 < bz> yeah
 22:43 < bz> Which is what images do now
 22:43 <@vlad> ok
 22:43 < bz> They get the intrinsic size from the image itself
 22:43 < bz> are you reusing nsImageFrame?
 22:43 <@vlad> no
 22:43 < bz> Or using an nsCanvasFrame?
 22:44 <@vlad> nsHTMLCanvasFrame
 22:44 < bz> I guess nsImageFrame does a lot of junk you don't care about so much...
 22:44 <@vlad> verious people suggested to me that nsImageFrame does a lot of stuff with image loading that would be hard to handle
 22:44 < bz> yeah
 22:44 <@vlad> and noone really mentioned nsCanvasFrame, and i'm not really sure what it does
 22:44 < bz> Though the thought of a common superclass glimmers
 22:44 < bz> oh, sorry
 22:44 < bz> I forgot we have an nsCanvasFrame already
 22:44 <@vlad> in theory, a common superclass would wo.. yeah that
 22:45 < bz> That's the thing that paints the viewport background
 22:45 <@vlad> ah
 22:45 <@vlad> unfortunate naming clash
 22:45 < bz> So if we make the width/height be the intrinsic size
 22:45 < bz> In the common case we have compat with safari
 22:45 <@vlad> yeah
 22:45 < bz> Since the common case is that no CSS is applied
 22:45 <@vlad> and they can always update to this impl as well
 22:45 < bz> Has anyone tested what Safari does when CSS _is_ applied?
 22:45 <@vlad> no, i'll do so shortly though
 22:45 < bz> And yes, they can
 22:46 < bz> ok
 22:46 <@vlad> we still want hspace/vspace/border attributes, I'd assume
 22:46 <@vlad> so now
 22:46 <@vlad> when I do <canvas style="border: 5px solid black;"></canvas>
 22:46 <@vlad> I get no border
 22:48 < bz> right
 22:49 < bz> So let me look at your frame code.  ;)
 22:49  * vlad cringes
 22:49 < bz> OK
 22:49 < bz> So there's space for the border, right?
 22:49 < bz> But it's just not painted?
 22:50 < bz> huh
 22:50 < bz> gimme a sec
 22:50 <@vlad> haha
 22:50 < bz> When did this land?
 22:50 <@vlad> ~2-3 weeks ago
 22:50 < bz> I guess I can use a recent nightly...
 22:50  * bz is browsing with an April 14 build
 22:50 < bz> which is safe... ;)
 22:50 <@vlad> mm, yeah
 22:50 <@vlad> it landed but wasn't turned on until very recently, like in the last week
 22:52 < bz> ok
 22:52 < bz> looking
 22:53 < bz> so we have the right size
 22:53  * bz tries to recall the jungle of paint methods
 22:54 < bz> yeah
 22:54 < bz> you just never paint the border
 22:54 < bz> http://lxr.mozilla.org/seamonkey/sou...Frame.cpp#1299
 22:54 < bz> You want that chunk
 22:55 < bz> Down through right after PaintSelf()
 22:55 < bz> That paints background, border, and outline
 22:55  * bz assumes we do want to paint the background
 22:55 < bz> which is a bit of an assumption here... ;)
 22:57 <@vlad> we don't actually
 22:57 <@vlad> (sorry, was distracted)
 22:57 < bz> ok
 22:57 < bz> How come not?
 22:57 < bz> That is, if there is a CSS specified background, shouldn't it go under whatever is painted in the canvas?
 22:58 <@vlad> well...
 22:58 <@vlad> maybe, but the default should be transparent
 22:58 < bz> And more importantly, under the border
 22:58 < bz> sure
 22:58 <@vlad> yeah, true
 22:58 < bz> The default background in CSS is transparent
 22:58 <@vlad> ok, so we do need PaintSelf()
 22:58 < bz> Yeah
 22:58 < bz> You need it to get that focus outline thing working too.  ;)
 22:59 <@vlad> ah, that explains why that doesn't work ;)
 22:59 <@vlad> ok, more questions
 22:59 <@vlad> in a different vein now
 22:59 <@vlad> ideally, people should be able to handle mousedown etc. events on the canvas
 23:00 <@vlad> right now the default behaviour on clicking it is to select it (give it the light blue overlay)
 23:00 <@vlad> does that behaviour change if someone handles mousedown?
 23:01 < bz> yes
 23:01 < bz> if they preventDefault() it
 23:01 < bz> or return false
 23:01 < bz> Which is the same thing
 23:01 < bz> well, more or less
 23:01 <@vlad> ok
 23:02 <@vlad> so i'll leave that be then
 23:02 <@vlad> ok, thanks a ton!  I'll wiki this up in a bit
 23:02 <@vlad> though i'm not sure what topic it should go under
 23:03 < bz> If we don't have a "gecko docs, style related" topic, we should make one.  ;)
 23:04 < bz> I need to doc up the content state system too...
 23:04 < shaver> on devmo or wiki.m.o?
 23:04 < shaver> I'd love to get more internals stuff on devmo
 23:04 <@vlad> so what's the difference between devmo and w.m.o?
 23:04 <@vlad> now that we have both
 23:04 < shaver> yeah, so
 23:05 < shaver> I think w.m.o is going to be more for project docs
 23:05 < shaver> and devmo will be for technical docs</object>

Document Tags and Contributors

 Contributors to this page: VladVukicevic
 Last updated by: VladVukicevic,