On 6/12/13 1:22 AM, "Justin Mclean" <jus...@classsoftware.com> wrote:
>HI,
>
>> Well, I'm not sure it is only an issue with the test.
>
>From a quick look it the setStyle that's not causing everything to
>refresh. The extra validateNow fixed that but was also called when
>scrolling, editing etc etc slowing the ADG down.
Well, that's a correct observation of the symptom, but not the cause.
In my version of the test case, after you hit "grouped data" and "expand
all", if you were to pause the app and examine the renderers in question
in the debugger, they all have their invalidateDisplayListFlag set to
true. That is because the validateNow() that was removed was preceded by
an invalidateDisplayList() call which set the flag to true, but no code
ran to validate the renderer. Then subsequent calls to setStyle,
depending on the style property will not update because the framework will
not re-register the renderer for validation if it already has
invalidateDisplayListFlag==true (it assumes that means it is already
registered).
IMO, it is risky to leave the ADG's renderers in this state. Articles and
forum posts that I and others have written about invalidateDisplayListFlag
and the other lifecycle methods and properties pretty much state that you
should never find these flags set to true unless they are registered for
later validation. Your proposed solution fixes this symptom by forcing
the entire ADG to recalculate the renderers. But the question is, what
about some third-party who extended the renderer and added another style
not in this list? Or calls setStyle that only directly affects the
renderers, bypassing the ADG (like changing a type-selector for the
renderer class). Has anybody done this? We'll be breaking that person's
app. Will anybody do this? I honestly don't know.
I think there are several possible solutions and directions to explore.
Here's the list I thought of and my personal opinions, but I'm going try
to leave it up to you to decide what risks you want to take.
1) Leave the code "as-is" even without the this proposed change of adding
layoutDirection to the if statement. Rating -0.5 since I don't like
leaving invalidateDisplayList==true, but one could argue that nobody is
going to change layoutDirection late in the startup, and nobody will
notice that the flag is set to true otherwise. I haven't tried it, but it
expect that moving setStyle in the test up earlier will not cause any
problems because the expandAll will refresh all renderers
2) This proposed change of adding layoutDirection to this list of
styleProps. Rating: -0.4. It leaves the flag set to true and doesn't
allow for anyone else to workaround another symptom like if they have
their own custom style that isn't getting updated in the renderer.
3) Changing this styleProp "if" to a lookup on some mx_internal
object/map. Rating: -0.1. Again this leaves the flag set to true, but at
least gives someone a way to hack in other properties (plus the check will
be much faster).
4) Change AdvancedDataGridItemRenderer to always register for validation
when invalidateDisplayList() is called. Rating: 0.5. This will make the
renderer conform to the lifecycle "spec", but will simply result in
validateNow() being called on the renderer, which you were trying to
eliminate
5) Add a flag to drawItem() to know if it is called from within
makeRowsAndColumns and skip calling updateDisplayOfItemRenderer. Rating:
0.5 (if it works). I don't know if that will work or not, but I think
this is fundamentally what you are trying to do by removing the
validateNow() call. drawItem and updateDisplayOfItemRenderer looks like
it might be needed in other situations like keyboard selection where the
entire set of renderers is not refreshed, but it seems like it is
duplicate work when refreshing renderers in makeRowsAndColumns (which is
also used during scrolling).
So, it may be that some combination of #4 and #5 would create a "more
correct" solution with fewer or no side-effects.
Thoughts?
-Alex
>
>Then perhaps a simple solution would to be this? ie add layoutDirection
>to the styleChanged method.
>
> override public function styleChanged(styleProp:String):void
> {
> super.styleChanged(styleProp);
>
> if (styleProp == "sortFontFamily"
> || styleProp == "sortFontSize"
> || styleProp == "sortFontStyle"
> || styleProp == "sortFontWeight"
> || styleProp == "layoutDirection")
> {
> itemsSizeChanged = true;
> rendererChanged = true;
> invalidateProperties();
> invalidateDisplayList();
> }
> }
>
>Probably "direction" should also be in there.
>
>Justin
>
