Harbs opened a new issue #1171:
URL: https://github.com/apache/royale-asjs/issues/1171


   Currently SimpleCSSValuesImpl gets style values like so:
   1. returns the value from the underlying style if it exists.
   2. Walks up the parent tree and returns the parent value if it's inherited.*
   3. I uses a lookup for either css class names or Class names to get assigned 
style info
   4. The Class names are saved in compiled MXML and added to the 
SimpleCSSValuesImpl by a call to `ValuesManager.valuesImpl.init(this)` in any 
class which implements `IMXMLDocument`
   5. If the object is a class, it checks the qualified name and uses that to 
look up the styles.
   6. When it doesn't find the styles, it will walk up the super-tree and does 
the same for each of the super-classes.
   
   This has multiple issues:
   1. Getting styles can be fairly expensive. As mentioned by @estanglerbm 
https://lists.apache.org/thread/qb1x76ohszmllkflzrq3mg70gy20y6tv The style 
lookups can be made much more efficient.
   2. Requiring lookups based on qualified class names requires keeping these 
names around in minified code for every class in the application.
   3. There's no way to cache known values to make lookups easier. (related to 
1)
   4. The code is pretty convoluted and can be made cleaner.
   
   Additonally there's some issues identified here:
   https://github.com/apache/royale-asjs/issues/641#issuecomment-573836029
   
   An example of a compiled MXML lookup is this:
   ```
   0,
   1,
   "org.apache.royale.core.Application",
   function() {this["MozUserSelect"] = "none";
   this["MsUserSelect"] = "none";
   this["WebkitUserSelect"] = "none";
   this["margin"] = 0.0;
   this["padding"] = 0.0;
   this["userSelect"] = "none"},
   0,
   1,
   "org.apache.royale.html.Container",
   function() {this["alignItems"] = "flex-start";
   this["iBeadLayout"] = org.apache.royale.html.beads.layouts.BasicLayout;
   this["iBeadView"] = org.apache.royale.html.beads.ContainerView;
   this["iViewport"] = org.apache.royale.html.supportClasses.OverflowViewport;
   this["iViewportModel"] = org.apache.royale.html.beads.models.ViewportModel},
   ```
   
   Here's a potential improvement:
   1. Instead of using `"org.apache.royale.html.Container"`, the value can be 
written unquoted to refer to the class directly like we do for bead references. 
This will enable to compiler to minimize the references normally.
   2. This means that you can't add the class names to a hash in the 
`valuesImpl`.
   3. Instead, we can add to `IStyleableObject` `setStyle()` and `getStyle()` 
That would put the logic of saving the style information and retrieving it in 
the class itself.
   4. `setStyle()` and `getStyle()` would be instance methods, but it would 
save the information to the class so it's written once.
   5. Additonally, getStyle can cache info it gets from the super-class so 
subsequent lookups would not need to walk up the hierarchy again. **
   6. We might have additional methods for `saveCacheValue` and `getCacheValue` 
for optimizing lookups when walking up parents if it's known that it's safe to 
do so. These values would need to be saved in instances for it to work. The 
right balance of memory usage and speed optimization would need to be found for 
this.
   
   * It looks like there's an inefficiency in step 2 where it will walk up the 
tree multiple times if the value is not set anywhere up the tree. That's 
because both `getValue` and `getInheritedValue` is called recursively in 
`getInheritedValue`. The current parent should probably be saved as an instance 
variable to prevent having to walk up the tree many times.
   * * need to figure out if caching the super-values might be an issue for 
modules.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to