Summary:
When layout happens, left/top/right/bottom coordinates (and thus clip-*, too) are sometimes not at a pixel boundary for 2 reasons:
a) width/height in Flexbox are floats, and can contain any value, not just a whole pixel. E.g. style={{width: 3.1415926}} is perfectly valid
b) floating point arithmetics sometimes leads to values barely outside of pixel boundaries (width 18.0f can become 18.000001f when a sibling/child size changes).
a) is \"breaking\" screenshot tests, which slightly but differ from reference implementation
b) causes extra View updates/redraws for no reason
This patch is rounding DrawCommand bounds to a whole pixel to avoid these 2 issues.
Reviewed By: ahmedre
Differential Revision: D2934401
Summary:
Public
We wanted to do this before, but couldn't because touch events had timestamps
set by the system (and matched System.currentTimeMillis), but now we set those
timestamps! The idea behind this change is that System.currentTimeMillis is
unreliable, but nanoTime isn't, and it also guarantees that we will never have two events with the same
timestamp.
We're still seeing crashes with touch events not ending correctly in JS, this
might be the cause of that.
Reviewed By: foghina
Differential Revision: D2953917
Summary:
Many of StateBuilder methods take a lot of arguments, which makes it hard to read. This patch is doing a bit of cleanup by removing tag from the method signatures, because tag can be easily obtained from the node itself.
This is a pure refactoring diff and should have no functional changes.
Reviewed By: ahmedre
Differential Revision: D2915815
Summary: Both DrawView and AbstractDrawCommand have clipping logic, this diff is moving out logic into a common base class. This also reverts the screenshot tests fix, which was causing issues with overflow: visible.
Reviewed By: ahmedre
Differential Revision: D2933780
Summary:
The current AndroidView stipulates that the backing shadow node can't
be a FlatShadowNode. In some cases, however, we want to apply some of the same
logic (ex not adding NodeRegions, etc) to other ViewManagers that have a
FlatShadowNode backing (and that don't necessarily create a FlatViewGroup).
This commit renames AndroidView to NativeViewWrapper, and re-introduces
AndroidView as an interface, so that logic for padding, NodeRegions, etc can
be shared.
Differential Revision: D2942387
Summary: Not every CSSNode in a hierarchy is a FlatShadowNode, some virtual nodes can be ReactShadowNodes for compatibility with ART nodes. This diff fixes StateBuilder unconditionally casting a node to FlatShadowNode, which is causing a crash in Groups.
Reviewed By: nickholub
Differential Revision: D2941031
Summary: These methods (mostly getters/setters) are not meant to be overriden, so let's make them final. Hopefully, they will inline better, too.
Reviewed By: ahmedre
Differential Revision: D2933869
Summary:
@public Relax the constraint on ReactTextInputManager.
The TextInput Advanced screen looked different with and without
nodes, namely child Text items were not being rendered on the Nodes version.
This patch fixes that.
Differential Revision: D2930800
Summary: FlatUIImplementation.handleCreateView was missing an Override annotation. This diff fixes it.
Reviewed By: sriramramani
Differential Revision: D2919596
Summary: Currently, we wrap all unknown shadow nodes with AndroidView. This works great, except when the shadow node is virtual, i.e. it *doesn't* mount to a View. In this case, we just need to keep it in the hierarchy as is. Fixes ARTSurfaceView not working correctly in groups.
Reviewed By: ahmedre
Differential Revision: D2933325
Summary: Software Canvas uses ints to represents boundaries (because it is backed by Bitmap that has scalar size), whereas Hardware Canvas uses floats to represent boundaries. This results in a bug in Software Canvas where clipping with floats close to or outside of int range results in int overflows and incorrect clipping. To fix the issue, compute the clip boundaries manually instead of using Canvas.clipRect() method (that contains the bug).
Reviewed By: sriramramani
Differential Revision: D2919509
Summary: There is a bug in border drawing code that will not draw the border if the element has a background color. This diff fixes it.
Reviewed By: sriramramani
Differential Revision: D2919549
Summary: ImageRequestHelper was not handling data: scheme correctly, which resulted in images failing to load. This diff is fixing it by considering \"data:\" as a Uri resource, and piping it appropriately.
Reviewed By: sriramramani
Differential Revision: D2919403
Summary: Previously, RCTImageView/DrawCommand would always clip for specific scale types (such as CENTER_CROP), but this is incorrect if we want to allow overflow: visible on RCTImageView (which iOS allows). This patch tweaks the clipping condition from paddingLeft/paddingTop < 0 (i.e. image wants to draw outside of its bounds) to actual image rect being outside of clip rect. Previously, that would mean exact same thing, but now that clip rect is aware of overflow: visible, it matters, allowing RCTImage to not clip when we want it to draw outside of the boundaries.
Reviewed By: ahmedre
Differential Revision: D2873350
Summary:
Previously, every node would be clipped by its own boundaries. This made sense, because
a) RCTView cannot draw outside of its bounds, so clipping is harmless
b) all other node types were not allowed to control overflow behavior, and for them overflow was implicitly set to hidden, which means it should clip.
This diff changes the logic so that overflow can be set on any node (image, text, view etc).
The change has an important implication that make the diff a little trickier that it could be: AbstractDrawCommand clipping logic needs to be relaxed. Previously, clip bounds were *always* intersected with node bounds, which means that clip rect was always within node rect. This is no longer true, and thus shouldClip() logic needs to be modified.
Because of the above, RCTText no longer needs to artificially clip bounds against layout width/height.
Note: RCTImage is still not working with overflow: visible correctly (it still always clips). This is fixed in a follow up diff.
Reviewed By: ahmedre
Differential Revision: D2867849
Summary:
There is an assert in FlatViewGroup.reactTagForTouch() that says that TouchTargetHelper should not allow returning getId() when pointer events are BOX_NONE. This is not entirely accurate.
This acturally method is invoked in 2 different contexts. Main context is to find a touch target, and in that context the method indeed should never return getId() if pointer events are BOX_NONE. There is however a TouchTargetHelper which actually expects that reactTagForTouch() *may* return getId(), in which case it will perform logic to not all this method be invoked again from main context.
In other words, this assert needs to be removed because it is entirely possible to return getId() when pointer events are BOX_NONE. Ideally, these would be 2 different methods, but ReactCompoundView interface only defines a single reactTagForTouch() method.
Reviewed By: ahmedre
Differential Revision: D2873931
Summary: When padding is applied to RCTText, the text it renders should be offset by that padding. This is what iOS is doing.
Reviewed By: sriramramani
Differential Revision: D2872039
Summary: A simple refactoring diff that should make code a little easier to read. No functional changes.
Reviewed By: ahmedre
Differential Revision: D2867718
Summary: Exact same class is needed for Groups, and probably for other apps that want to work with Nodes, too, so let's extract it into a helper class to simplify coding.
Reviewed By: sriramramani
Differential Revision: D2873943
Summary:
Nodes create a FlatViewGroup for a root node element, and NativeViewHierarchyManager assigns it an id that matches root node it. Now this top-level FlatViewGroup is added to a ReactRootView. In non-nodes, this ReactRootView has an id that matches root View id, and when this ReactRootView gets detached from Window, it sends an event to NativeViewHierarchyManager that says that all the children of this View must be removed recursively. This works great in non-Nodes, but fails in Nodes because ReactRootView has no id (-1) and NativeViewHierarchyManager fails to find a corresponding ReactShadowNode, throwing an Exception. To fix the issue and make it work again we need to assign this View id of the top level shadow node. This creates a minor issue, where 2 Views (ReactRootView and its only child) share the same id, but that is not a problem because we don't enforce uniqueness of the ids, and don't rely on getViewById().
This was originally implemented, but then I removed it because I thought it wasn't truly needed. Turns out, it is needed.
Reviewed By: sriramramani
Differential Revision: D2873995
Summary: This diff implements ImageLoadEvents (ON_LOAD, ON_LOAD_END and ON_LOAD_START) in RCTImageView. ON_ERROR and ON_PROGRESS are easy to implement, too, but these 2 are supposed to carry extra information (error message and progress) but ImageLoadEvent doesn't support a payload yet (I'll add them in a separate patch).
Reviewed By: ahmedre
Differential Revision: D2824772
Summary: React provides properties to support shadows for Text, this diff implements it for Nodes.
Reviewed By: ahmedre
Differential Revision: D2817212
Summary: Minor optimization/cleanup: instead of creating an instance of FontStylingSpan for every RCTVirtualText, start with a global immutable instance of use copy-on-write when any of the properties change.
Reviewed By: ahmedre
Differential Revision: D2817156
Summary: Wrong order or top/right -> right/top means that clipping can be incorrect. I didn't notice it because visual result in many cases would still be the same (especially with overflow: visible being default and clipping being mostly disabled). Also only background was clipped incorrectly, and most texts don't have a background.
Reviewed By: ahmedre
Differential Revision: D2816432
Summary: Small optimization that prevents empty NodeRegions from being collected. Those will never receive touch events anyway, so why bother allocating memory and iterating over them?
Reviewed By: ahmedre
Differential Revision: D2816355
Summary: RCTText may modify its DrawCommand on collectState(), which updateNodeRegion() relies on. This means that order of the two is important: call collectState() first, followed by updateNodeRegion(). This fixes the bug where am RCTText may sometimes not respond to clicks (because its NodeRegion would be empty).
Reviewed By: ahmedre
Differential Revision: D2816295
Summary: NodeRegions are touch regions within hosting View, and while in most cases they are the same as View boundaries, there is one case where it's not true: TextNodeRegion. When mounted to a View, TextNodeRegion will have a bounds of (0,0,width,height) which is clearly different from (left,top,right,bottom). Initially I assumed they would always be the same so we could use information stored in NodeRegion (should probably be called TouchRegion) to update node's View boundaries, but it breaks RCTTextView when it mount to a View (because it would either contain incorrect bounds, or View will be laid out incorrectly). Right now touch is not working on RCTView that mounts to a View. To fix the issue, separate the 2 concepts.
Reviewed By: ahmedre
Differential Revision: D2816268
Summary: Small optimization. Once we mount a Node to a View, its NodeRegion is not used anymore with RCTText being an exception, but it will correct itself automatically upon updateNodeRegion call. This patch allows the dead object to get garbage collected.
Reviewed By: ahmedre
Differential Revision: D2816331
Summary: Alternative implementation of DrawImage using DraweeHierarchy instead of ImagePipeline directly. Yields same results, but potentially more stable. We'll run tests to measure performance of both.
Reviewed By: ahmedre
Differential Revision: D2746197
Summary:
Moves ReactProp and ReactPropGroup to `com.facebook.react.uimanager.annotations`. This is needed
so that future annotation processor can run on code inside the com.facebook.react.uimanager package.
@public
Reviewed By: astreet
Differential Revision: D2754842
Summary: View.invalidate() will not actually invalidate a View if it has width or height equal to 0. This is causing problems with overflow: visible. A quick fix applied here is to invalidate slightly larger region that the View bounds to bypass the optimization.
Reviewed By: ahmedre
Differential Revision: D2800920
Summary: Virtual shadow nodes, such as RCTRawText, RCTTextInlineImage etc cannot be mounted to a View (ViewManager will throw an Exception if we ever try to). This diff is adding a check to make sure that Exception is never thrown.
Reviewed By: ahmedre
Differential Revision: D2800869
Summary: UIImplementation.measure() can only measure shadow nodes that map to native Views. As a result of this, every time we tried to measure a shadow node that doesn't map to a View, we trigger callback with no data (to indicate an error). To fix this issue, walk up until we find a node that maps to a View, then measure that View and adjust for the bounds of a virtual child.
Reviewed By: ahmedre
Differential Revision: D2800960
Summary:
Everything (but Views) are drawn using AbstractDrawCommand (or its derivative, like DrawBorder or DrawBackgroundColor) and supports clipping properly. Views however are drawn using an assumption that Android will clip them manually. This is however not always true. One example is if an element A mounts to a View, and its parent B doesn't but has overflow: hidden and thus should clip the child.
There are 2 ways to fix this:
- pop every element that has overflow: hidden to its own View. In this case, its children will always be correctly clipped. This is however very inefficient, especially if overflow: hidden is default (which it is!) which means that almost every React element must be backed by an Android View.
- add clipping information to DrawView, similar to how AbstractDrawCommand has it.
This diff implements the latter approach.
Reviewed By: ahmedre
Differential Revision: D2792375
Summary: RootFlatShadowNode always mounts to a (top-level) View but was never marked as one that mounts to a View. This diff fixes it. Nothing in the code relied on it being marked as mounting to a View, so it worked fine. A follow up path that implements UIImplementation.measure() for nodes that don't mount to a View goes up until it finds first mounting node, and it was crashing with NPE because it tried to access RootFlatShadowNode's parent (and there isn't one).
Reviewed By: sriramramani
Differential Revision: D2800818
Summary:
React allows nesting <Image> inside <Text>, in which case it turns into an RCTTextInlineImage element. RNFeed is using it in a few places and thus we need to support it, too.
This diff implements it with InlineImageSpan (WithPipeline, and WithDrawee separately).
Reviewed By: ahmedre
Differential Revision: D2792569
Summary: D2768643 disabled View clipping of FlatViewGroup's children by calling setClipChildren(false). Turns out, instead of disabling clipping on the View this method is called on, it disables clipping on the children, instead. While we want the clipping disabled on all children that are FlatViewGroups, it also means that everyone else gets the clipping disabled as well. This has issues with ScrollView that stopped clipping its scrolling content, resulting in visual glitches. To fix the issue, manually clip all the Views that are not FlatViewGroups.
Reviewed By: ahmedre
Differential Revision: D2792411
Summary: This diff add an ImageRequestHelper class to reuse code that creates ImageRequest in both RCTImageView and (upcoming) RCTTextInlineImage
Reviewed By: ahmedre
Differential Revision: D2792548
Summary: Minor cleanup; renames BitmapRequestHelper to PipelineRequestHelper and adds BitmapUpdateListener interface. I plan reusing these classes in an upcoming RCTTextInlineImage implementation.
Reviewed By: ahmedre
Differential Revision: D2792535
Summary: AbstractDrawCommand's bounds are supposed to be the rectangle that the DrawCommand will draw into, so that it can compare bounds against clip rect bounds and reliable skip clipping when it's not needed. It is however not true for RCTText that can create text Layout that in some cases can go out of it's requested bounds (happens with TextView, too). To fix the issue, instead of passing requested bounds, pass the actual ones. In this case, AbstractDrawCommand will make sure to clip DrawTextLayout if for some reason it draws outside of the allowed boundaries.
Reviewed By: ahmedre
Differential Revision: D2786306
Summary:
@public React allows excluding certain elements from touch handling by assigning `PointerEvents` filter to them, such as BOX_NONE - this element will not receive touch but its children will, BOX_ONLY - only this element will receive pointer event and not children, NONE - neither this element nor its children will receive pointer events, and AUTO - pointer events are allowed for both this element and its children.
This diff adds PointerEvents support to flat RCTView. Most of the implementation is copied from ReactViewManager/ReactViewGroup. One small change is made to TouchTargetHelper to ensure that it works correctly with virtual nodes when their parent has PointerEvents set to PointerEvents.BOX_NONE.
Reviewed By: ahmedre
Differential Revision: D2784208
Summary: There is a small bug in DrawBorder where we forget updating color for a left border, which makes the border draw with whatever color was previously set to the Paint. This diff fixes it by updating the color.
Reviewed By: ahmedre
Differential Revision: D2779511
Summary: When setJSResponder() is called on a shadow node that doesn't mount to a View, React runtime will crash in NativeViewHierarchyManager because it will fail to find a corresponding View. To fix the issue, make sure we forceMountToView() before we call enqueueSetJSResponder().
Reviewed By: ahmedre
Differential Revision: D2779523
Summary: D2772438 removed some unused imports, and a parallel build landed that uses one of the imports. Both diffs landed and now master is broken. This diff is fixing it.
Reviewed By: ahmedre
Differential Revision: D2779442
Summary:
Prior to this patch DrawCommands weren't get clipped by parent DrawCommands at all. For example, a <View> element with size 200x200 with overflow:hidden should clip its child which is 400x400, but this didn't happen. However, if parent <View> would mount to an Android View, it would clip the child regardless of the overflow attribute value (because Android Views always clip whatever is drawing inside that View against its boundaries).
This diff is fixing these issue, implementing overflow attribute support and making clipping behavior consistent between nodes that mount to View and nodes that don't.
Reviewed By: ahmedre
Differential Revision: D2768643
Summary: I lost FlatViewGroup.verifyDrawable(Drawable) method during one of the rebases. Re-adding it now because hotspot drawable is not working without it.
Reviewed By: ahmedre
Differential Revision: D2772460
Denis Koroskin
Andrei Coman
Ahmed El-Helw
Martin Konicek
Alexander Blom