[Webkit-unassigned] [Bug 102958] [chromium] make compositor request the font atlas #1

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=102958





--- Comment #9 from James Robinson <jamesr at chromium.org>  2012-11-26 11:09:45 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > (From update of attachment 175486 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=175486&action=review
> > 
> > > Source/Platform/ChangeLog:3
> > > +        [chromium] make compositor request the font atlas #1
> > 
> > This does not describe the patch or the bug and is grammatically iffy.  Please rephrase
> 
> Yeah, I found it pretty hard to find something descriptive. What would be better? 
> 
> - replace font atlas passing with requesting
> - make font atlas creation accessible to WebLayerTreeView and remove font atlas passing
> - replace WebLayerTreeView::setFontAtlas() with WebLayerTreeViewClient::createFontAtlas()
> 
> I tried to find one name for all three patches and number them. Is that a good idea? I need some help here please.

That sounds like a good list to have in the ChangeLog.  For the top line, you could describe the API change this adds - for instance something like "[chromium] Add WebLayerTreeViewClient API to request font atlas" or something like that.


> 
> > > Source/Platform/chromium/public/WebLayerTreeViewClient.h:109
> > > +    virtual void createFontAtlas(SkBitmap&, WebRect[128], int&) { }
> > 
> > The third parameter isn't very clear from the signature - could you provide a descriptive name?
> 
> I had a look at how this was done before, the second parameter was named as well. Is this better?:
> 
> virtual void createFontAtlas(SkBitmap&, WebRect asciiToRectTable[128], int& fontHeight) { }

Yes

> 
> Is there a rule of thumb of when to name parameters in interface methods?

Yes - if the name of the parameter is fairly obvious from the type and signature, leave it out.  Otherwise, include it.  For instance having the first parameter be called 'bitmap' would be redundant with the type and calling it 'atlas' would be redundant with the function name, so leaving it out is fine.  See http://www.webkit.org/coding/coding-style.html#names-verb

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.