On 12/26/13 2:25 AM, "Justin Mclean" <[email protected]> wrote:
>Hi, > >Thanks for looking into it. > >> Found some time to look. Four of the five show antialiasing >>differences. >I checked in photoshop by putting the pairs of images on layers and doing >a difference blend and there were no obvious changes. There is a mustella/utilities/ImageDiffAIR that will show you every pixel difference, no blending required. It can catch anti-aliasing differences because of lack of embedding fonts, etc. I guess it is time for me to try to hook it up to some ant or shell script to make it easier to build and run and use. > >> I'm not convinced the fonts are embedded correctly in these tests. >How do we check that? One way is to use the checkEmbeddedFonts mixin (-addArg=-includes=CheckEmbeddedFonts) for these tests since I think they use TextField under the covers. Another is to stop in the debugger on CompareBitmap.as:execute and examine each textfield. > >> One test (messageStyleName) seems to drop the last line of text? >In this case I think the current code is wrong as the text goes over the >buttons. I agree the current code is wrong, but I think the fix should have just made the Alert wider or taller so all text is shown. > >> One other(messageEmbedFont) has the text at a slightly different y >>position as you >> mentioned. >Which I'm not sure why, given that all of the other tests have the text >in the right position. > >> Seems like some other logic may be busted in Alert. >Quite possible as previously the text could exceed the window bounds. > >> Might be a bigger mess that requires more than just this patch. >But this is an improvement of what already exists right? It occurred to me that caching textHeight in measure and using it in updateDisplayList is a questionable practice. Someone could forcibly resize the Alert in which case measure() would not be called again and updateDL would use a stale value, so I don't think the proposed patch is going to be an improvement in all cases. I was wondering whether the un-patched code simply shouldn't cache textHeight as it misleads folks into thinking it should be used in updateDL. And then there is some other change needed so the Alert gets better at growing when there is more text. Yes, it will cap at the size of the screen and then other logic is needed to not overlap the buttons, but caching textHeight may not be the right thing to do here because the Alert can be resized manually. > >Thanks, >Justin
