Commit Graph

909 Commits

Author SHA1 Message Date
Emil Sjolander
12ebaa56b6 Dont measure single flex grow+shrink child
Summary: If there is a single child which is flex grow and flex shrink then instead of measuring and then shrinking we can just set the flex basis to zero as we know the final result will be that the child take up all remaining space.

Reviewed By: gkassabli

Differential Revision: D4147298

fbshipit-source-id: 51152e57eff8e322a833a6d698c30f8c5e2dcc35
2016-11-08 15:37:48 -08:00
Emil Sjolander
3e567fdcae Fix flex within max width constraint
Summary:
Max dimension constraints were not correctly passed down to children.

see https://github.com/facebook/css-layout/issues/230 for more info
fixes #230

Reviewed By: gkassabli

Differential Revision: D4147199

fbshipit-source-id: feb335eb8687a1b7939ee8cd8649e455e0c069a9
2016-11-08 09:53:00 -08:00
Emil Sjolander
1baa239389 Remove isTextNode optimization
Summary: Scrolling through feed and logging when this optimization is hit leads to... 0 logs. If anything this just adds to confusion. It was initially added due to instinct and not data, which was a mistake. I am happy to add some similar optimization in the future if we have data that it is useful in real world situations, currently it just leads to bugs and confusion though.

Reviewed By: astreet

Differential Revision: D4146785

fbshipit-source-id: e20d780fbd5759b8f38b809e8cadf29cedee82a8
2016-11-08 09:22:38 -08:00
Dustin Shahidehpour
b50090a04e NSAssert -> NSCAssert.
Summary: Not sure why this didn't catch during my testing, but, this causes a build error. #accept2ship

Reviewed By: emilsjolander

Differential Revision: D4143108

fbshipit-source-id: 01c35c5b91767c95485d615eb06e836b023e125a
2016-11-08 07:22:48 -08:00
Kazuki Sakamoto
ba2905dd1d Add C# test script for macOS
Summary: - Updated README and added bash script for testing C# on macOS

Reviewed By: emilsjolander

Differential Revision: D4142507

fbshipit-source-id: c682778e42d10242e02f3fdafe8a23f906bfabc5
2016-11-07 19:37:57 -08:00
Dustin Shahidehpour
997088ffbb Fix broken unit tests.
Summary: For some reason I didn't get a signal of this breakage. Fixing unit tests.

Reviewed By: emilsjolander

Differential Revision: D4140327

fbshipit-source-id: 95049b510a8869b6f68cc841e7f8731364417af9
2016-11-07 14:37:56 -08:00
Emil Sjolander
ffb90e6ff2 Remove check for both modes being exact as this will never happen
Summary: css-layout will only call measure which it is unsure of the size of a node so it will never call measure with both modes equal to exact.

Reviewed By: dshahidehpour

Differential Revision: D4142193

fbshipit-source-id: fb8423cf0d45852ecb9df8fed042b25d1a443eea
2016-11-07 14:37:55 -08:00
Rachit Siamwalla
1c22a1aa53 Fix xplat CSSLayoutTests to compile on platforms without death tests.
Summary:
Not all platforms have gtest death tests. There is a define to check for that.
Bracketed the relevant tests with the define.

Reviewed By: youerkang

Differential Revision: D4141823

fbshipit-source-id: 1396657f3ee83853fcda85d5a51708d4e77642cb
2016-11-07 14:07:49 -08:00
Dustin Shahidehpour
7082734c6b Kill border API.
Summary: We don't want to expose this in our first usage. Lets kill it.

Reviewed By: emilsjolander

Differential Revision: D4140377

fbshipit-source-id: 0c53845ec65466692b847a5ce40c3b9823dd9557
2016-11-07 13:37:47 -08:00
Dustin Shahidehpour
366a61af8d Take care of pixel rounding for UIView.
Summary: You can lose a lot of performance on UIView's if they are not pixel-aligned. This protects it from happening in views which use css-layout.

Reviewed By: emilsjolander

Differential Revision: D4140306

fbshipit-source-id: 53493c08c084a7e3dcd4e05f6a665a99340ea5a6
2016-11-07 13:37:47 -08:00
安秋亮
18fe0959f0 Add inline definition for Microsoft Visual Studio
Summary:
The inline keyword is available only in C++. The __inline and __forceinline keywords are available in both C and C++. For compatibility with previous versions, _inline is a synonym for __inline.

https://msdn.microsoft.com/en-us/library/z8y1yy88(v=vs.120).aspx
Closes https://github.com/facebook/css-layout/pull/239

Reviewed By: astreet

Differential Revision: D4138941

Pulled By: emilsjolander

fbshipit-source-id: cb59dc91ef285e5378036c4912217fd4ec8d9f79
2016-11-07 13:07:40 -08:00
Dustin Shahidehpour
1ffce3edb1 Sanitize results from sizeThatFits:, fix bug where only one of the measure modes is CSSMeasureModeExactly.
Summary: As I've begun to integrate this into the codebase, I've found that sometimes our layouts are affected by bad implementations of sizeThatFits:. For example, in certain configurations of UILabel it won't take clipping into account and return a size that is much larger than the max size we requested. This adds some checks to make sure we never return a size that is larger than the one specified by `_measure`. This also fixes the bug where `CSSMeasureModeExactly` won't be applied if the measure mode for other parameter is node `CSSMeasureModeExactly`.

Reviewed By: emilsjolander

Differential Revision: D4131195

fbshipit-source-id: 4659fd83892a2c149b3b70733b06b19217507bf9
2016-11-07 06:52:47 -08:00
Andy Street
6165d7a2be Make CSSNode#measure final, cache more correct methodid
Summary:
@public

In the JNI portion of CSSLayout, there's a subtle bug where we were caching the jmethodid of the 'measure' of the first object that had measure called on it. However, if that class had overriden measure, then the jmethodid would be specific to that subclass's implementation and would not work for other classes. Conversely, if a regular CSSNode had been called first, then the super method would be called on the subclass instead of the proper overriden method.

Since there's not really a reason to overriden measure anyway (since you can just provide a different measure function), it's safest to just mark it final and explicitly cache the appropriate methodid

Reviewed By: emilsjolander

Differential Revision: D4132428

fbshipit-source-id: 6fb51597d80f1c03681e6611153b52b73b392245
2016-11-07 06:07:36 -08:00
Andy Street
01a3881426 Lazy create children list when first child is added
Summary:
@public

We don't need to allocate a list for every node since leaf nodes don't have children.

Reviewed By: emilsjolander

Differential Revision: D4130818

fbshipit-source-id: 80d3e98fce9d2daa0676fd1cbed0e81edcf7adb3
2016-11-07 05:52:41 -08:00
Andy Street
9c1896043d Assert that node can have measure function <==> node is a leaf node
Summary:
@public

Instead of silently ignorning non-leaf nodes with measure functions, we should assert that we don't create those kinds of trees.

Reviewed By: emilsjolander

Differential Revision: D4130770

fbshipit-source-id: a3ef10a2e63bbc12b5aa07977e4b84c8d59e3ffe
2016-11-07 05:37:45 -08:00
Dustin Shahidehpour
0f822bbef2 Remove overflow and flex from API.
Summary: flex is just shorthand for flexBasis, flexGrow and flexShrink, and I don't want to expose overflow yet, so, removing them from the API.

Reviewed By: ryanolsonk

Differential Revision: D4126773

fbshipit-source-id: e3b9ef714832cb5665bd20d7fa92f97a266c9210
2016-11-04 13:37:40 -07:00
Dustin Shahidehpour
e00e30ca15 Only mark Nodes dirty if an actual node is removed.
Summary: Currently, when we try to remove a child from a node, that node is mark dirty //regardless of whether or not anything was actually removed//. This fixes it.

Reviewed By: gkassabli

Differential Revision: D4125453

fbshipit-source-id: 745cfc55269415fea106a80c72401eb3074f2d31
2016-11-03 13:38:05 -07:00
Dustin Shahidehpour
b938017ccf Add Sizing API that doesn't change view's frame.
Summary: This exposes an API so people can find out the resulting size of a layout calculation without changing the frame of the view.

Reviewed By: emilsjolander

Differential Revision: D4124630

fbshipit-source-id: f2b28d8a5474857cb1c92e021a1f161806826cda
2016-11-03 13:07:37 -07:00
Georgiy Kassabli
630ae0972b Exposing layout cache check publicly
Summary: We need to expose CSSLayout caching check to CKFlexboxComponent to enable performant bridging to CKComponentKit

Reviewed By: emilsjolander

Differential Revision: D4124705

fbshipit-source-id: 23284967900585fa20dcb51c9cc1bee829b32975
2016-11-03 10:52:43 -07:00
Dustin Shahidehpour
d8662805d5 Fix incorrect results from css_usesFlexbox
Summary: With the current implementation, as long as you had set a value on usesFlexbox (YES or NO), it would return YES. This fixes it.

Reviewed By: amonshiz

Differential Revision: D4124485

fbshipit-source-id: 32fe4ec0109b5c8678a112f9d4295bd17a6f6dc2
2016-11-03 09:37:41 -07:00
Dustin Shahidehpour
26e7490fc9 Make CSSNodeRef readonly.
Summary: We don't want people to somehow change the underlying cnode on our CSSNodeBridge. Making it readonly.

Reviewed By: rnystrom

Differential Revision: D4124496

fbshipit-source-id: f88ed82c1df17d401aeca6f6341e429c95e55624
2016-11-03 09:07:37 -07:00
Dustin Shahidehpour
d94363ea7e Cleanup Measure method.
Summary: We have redundant/dead code in UIKit's sizing method, lets clean it up.

Reviewed By: emilsjolander

Differential Revision: D4110939

fbshipit-source-id: 35e856aa6c60fd24316bc67cc564f7eec2d9d714
2016-11-02 17:07:47 -07:00
Lukas Wöhrl
af5e6771d7 Prevent array out of bound access
Summary:
Even so the problem of #236 has been fixed via ced779b there is still a case where you have a out of bound array access of undefined behaviour. This PR prevents the out of bound access of the underlying c array. The out of bound access happens if you already have 4 subViews and add another one. Then you will access CSSNodeGetChild with 4 which points into uninitialized memory.
Closes https://github.com/facebook/css-layout/pull/237

Reviewed By: dshahidehpour

Differential Revision: D4107743

Pulled By: emilsjolander

fbshipit-source-id: 0f21397f3a77308369acfea7327733f74eb72e00
2016-11-01 20:54:00 -07:00
Dustin Shahidehpour
ced779b259 Prevent crash when accessing child count, but child list is NULL.
Summary: Previously, we would preallocate Node's with a child list of 4. We recently removed that logic (see diff below), and as a result, if you tried to access a Node's list of children before it had been allocated, you would crash. I added a simple check to protect from crashes, the operation of the check is O(1) so we shouldn't see a perf hit.

Reviewed By: emilsjolander

Differential Revision: D4104093

fbshipit-source-id: cd7b09818759aa76415b97e241f1a6746a2bc50c
2016-10-31 12:52:52 -07:00
Emil Sjolander
a65e6930cf Some small simplifications for function return values
Summary: Simplify logic for what value to return in smaller functions with a preference for ternary operator where possible.

Reviewed By: gkassabli

Differential Revision: D4101772

fbshipit-source-id: 626df10c0fc76278c330c86be4dc82fdda5f5156
2016-10-31 11:08:05 -07:00
Emil Sjolander
267e17f23a void* -> CSSNodeRef in CSSNodeList
Summary: This list can only contain CSSNodeRefs so don't use void*

Reviewed By: gkassabli

Differential Revision: D4101773

fbshipit-source-id: 8a5c9066da796967bd02ce6b1fc74ff40e15dfe1
2016-10-31 11:08:02 -07:00
Emil Sjolander
df6368e048 Add complex layout to benchmark
Summary: Add a more complex benchmark which actually take a couple milliseconds to perform. This makes it easier to see if optimizations have any effect. More styles should be added later to make sure the benchmarks covers most of the csslayout code.

Reviewed By: gkassabli

Differential Revision: D4101780

fbshipit-source-id: 6bdf703edfbe64c47c77e04ef1ca946f4b75d093
2016-10-31 10:38:02 -07:00
Emil Sjolander
ba20d75992 Fix benchmark
Summary: Benchmark accidentally broke during a refactor

Reviewed By: gkassabli

Differential Revision: D4101776

fbshipit-source-id: 00094837b7856c06c0463c48929f960122c0c8ac
2016-10-31 10:38:02 -07:00
Dustin Shahidehpour
620cb3f507 Remove static method declarations.
Summary: Since these functions are private and only used in the implementation file, we don't need to declare them beforehand.

Reviewed By: emilsjolander

Differential Revision: D4088488

fbshipit-source-id: 738175a4aae27d88d32f8c2cf6b47a4f6ae32aab
2016-10-27 12:52:43 -07:00
Kazuki Sakamoto
3201e24780 Fix build and clean up
Summary:
- bit operation with long
- Clean up _measureOutput which is no longer needed
- Fix unittests (SetMeasureFunction, unused error)

Reviewed By: emilsjolander

Differential Revision: D4082401

fbshipit-source-id: b3b2dd002d108c5b43f36a4a73ce8e5233281b45
2016-10-27 11:53:11 -07:00
Emil Sjolander
2cac77eaa1 Set layout outputs on java object from C
Summary: Set layout outputs on CSSNode from C after layout calculation finishes instead of relying on calling jni gettings from java code. This should be much more efficient as it avoids a lot of jni overhead and also allows for calling getLayoutWidth() etc multiple times without incurring a penalty.

Reviewed By: lexs

Differential Revision: D4077968

fbshipit-source-id: bce86ba610cd5ae36cfb45d78b2609c63a14cfa3
2016-10-27 10:52:57 -07:00
Emil Sjolander
46823878a5 BREAKING - Make first parameter of measure and print functions CSSNodeRef instead of just context
Summary: To perform some JNI optimizations for java we need a reference to the node in the measure function. This updates the API to provide the whole node as input instead of just the context.

Reviewed By: javache

Differential Revision: D4081544

fbshipit-source-id: d49679025cea027cf7b8482898de0a01fe0f9d40
2016-10-27 10:52:57 -07:00
Emil Sjolander
b59ce09109 BREAKING - Change measure() api to remove need for MeasureOutput allocation
Summary: This is an API breaking change done to allow us to avoid an allocation during measurement. Instead we do the same trick as is done when passing measure results to C, we path them into a long.

Reviewed By: splhack

Differential Revision: D4081037

fbshipit-source-id: 28adbcdd160cbd3f59a0fdd4b9f1200ae18678f1
2016-10-27 10:52:57 -07:00
Emil Sjolander
c34299edc9 Reset java state in jni reset method
Summary: This state was never reset when we switched to doing reset in C instead of re-allocating.

Differential Revision: D4081049

fbshipit-source-id: 0b9ad70339ad906ad5219ad2679329cfe2fd7abc
2016-10-26 07:37:47 -07:00
Kazuki Sakamoto
1ba81d9ec7 Prevent GC and avoid new
Summary:
- Prevent the GC from collecting MeasureInternal in order to call managed MeasureFunction properly from unmanaged
  - TestMeasureFuncWithDestructor will fail without the fix
- Avoid new as C implementation

Reviewed By: emilsjolander

Differential Revision: D4080765

fbshipit-source-id: d58fa18f6f74012aeda5dd15dfa7ceb0b64584d0
2016-10-26 06:52:41 -07:00
Emil Sjolander
d6d817c142 Dont go down through JNI to figure out that no margin/padding/border/position was set
Summary: Many layout systems query the padding after calculation to as it is needs to be propagated to the underlying view system on the platform. However most nodes have no padding set on them so going 4-6 times through JNI layer to figure this out is a waste of time.

Differential Revision: D4080909

fbshipit-source-id: 7eb1885c615191055aa21e3435c6fbc652b883ae
2016-10-26 03:07:42 -07:00
Scott Wolchok
01c2ac3369 Don't preallocate child lists
Summary: There is no reason to malloc a list of 4 child pointers for every CSS node eagerly. Instead, we malloc the list (preserving the default size of 4) when we try to put stuff in it.

Reviewed By: emilsjolander

Differential Revision: D4078012

fbshipit-source-id: 7cdcab03ec4067550a5fee5e1baea14344f3a8f9
2016-10-25 17:22:47 -07:00
Emil Sjolander
0cc1b83569 Remove nanosleep from benchmark measure and increase loop count
Summary: sleeping in the measure function was done to ensure we were not regressing in double measure calls. This was before we have CSSLayoutMeasureCacheTest though. Let's switch over benchmark to purely benchmark the algorithm and not simulate measure time.

Reviewed By: swolchok

Differential Revision: D4078186

fbshipit-source-id: e1c16806b3c8714e223e1cfcb6c43846f8f6bbb2
2016-10-25 16:23:04 -07:00
Lukas Wöhrl
c8d77b2aae Remove children from the end to prevent copying over
Summary:
This PR leads to removing the children from the end. [So we don't have the overhead of copying them to the front](dcaef7ecb0/CSSLayout/CSSNodeList.c (L62)).
Closes https://github.com/facebook/css-layout/pull/233

Reviewed By: emilsjolander

Differential Revision: D4075905

Pulled By: splhack

fbshipit-source-id: d8b460badb01bfc6ea647004c799395b9cab9e7c
2016-10-25 12:22:48 -07:00
Emil Sjolander
01507044b3 Suggest the compiler to inline smaller functions
Summary: Suggest the compiler inline some functions. Shows ~15% performance benefit on android compiling with gcc.

Reviewed By: gkassabli

Differential Revision: D4074580

fbshipit-source-id: 69b63fadf2011cb688af58f09d67c2cb711a0e20
2016-10-25 10:37:45 -07:00
Kazuki Sakamoto
2168d68007 Update CSSNodeFree for C#, Java and Objective-C
Summary:
- Update CSSNodeFree to unlink parent and children for C#, Java and Objective-C bindings finalizer.
- [C#] Fix build (Fix #232)
- [C#] Add Clear API for convenience as CSSNodeFreeRecursive.
- [C#] Revise and add unit tests

Reviewed By: emilsjolander

Differential Revision: D4069655

fbshipit-source-id: 1fd764059784d7968af38b6aaf7fb6f70fdee8ee
2016-10-25 07:52:39 -07:00
Emil Sjolander
4d0e657653 Fix bug in canUseCachedMeasurement causing unneeded double measure
Summary: canUseCachedMeasurement function was not handling CSSMeasureModeAtMost correctly so a bunch of measurements that could have been reused were not. When a previous measurement used AtMost and the new one as an AtMost with a smaller constraint but still bigger than the previous computed size it should be OK to re-use the previously computed size.

Reviewed By: gkassabli

Differential Revision: D4071621

fbshipit-source-id: 19d87d939051ddf8ee2e1c6e60efee22d173bbb9
2016-10-25 07:37:59 -07:00
Kazuki Sakamoto
4452c7be5c Remove no longer needed Spacing.cs
Summary: - Remove no longer needed Spacing.cs

Reviewed By: emilsjolander

Differential Revision: D4073539

fbshipit-source-id: 916cf85119c6bac3d516de5396ea3ba7b0af1475
2016-10-25 07:08:00 -07:00
Emil Sjolander
15f5c4cea5 Reduce duplicate function calls
Summary: Remove duplicate functions calls. Using instruments I could see a 5% perf increase from this change.

Reviewed By: gkassabli

Differential Revision: D4068140

fbshipit-source-id: 91261afb73e1c5e23c2cfc84df6ecc5c844a4e78
2016-10-24 12:37:49 -07:00
Emil Sjolander
97fef59f96 Dont hold strong reference to java objects creating a cycle
Summary: Use weak reference to avoid cycle

Reviewed By: splhack

Differential Revision: D4064773

fbshipit-source-id: 4088fef5e088a8415747898ef17851e21ada5180
2016-10-24 12:37:49 -07:00
Emil Sjolander
e9b9973cae Dont create a spacing object for returning margin, padding, border, and position
Summary: The current implementation was made out of simplicity and to keep the same API as before. Now that the java version of csslayout is deprecated it is time to change the API to make the calls more efficient for the JNI version. This diff with reduce allocations as well as reduce the number of JNI calls done.

Differential Revision: D4050773

fbshipit-source-id: 3fd04c27f887a36875e455b5404a17154ac18f91
2016-10-24 10:37:51 -07:00
Emil Sjolander
69c374e74e Simplify memory model between managed and unmanaged memory
Summary: Instead of having different lifetimes for java and c memory we can can tie them together and make them much easier to manage. This also leads to automatically pooling native memory if pooling java memory.

Differential Revision: D4051454

fbshipit-source-id: 8f5d010be520b3d1c981a7f85e5e6d95773ea6c1
2016-10-24 10:37:51 -07:00
Emil Sjolander
4c57029a28 Remove flex shorthand getter because it doesnt make a lot of sense
Summary: It doesn't make sense to have a getter for the shorthand as it is the computed flexGrow and flexShrink values that you should care about.

Reviewed By: gkassabli

Differential Revision: D4064674

fbshipit-source-id: 69935b85042020b4e8c61a393c1be8f4d42a6674
2016-10-24 03:37:49 -07:00
Emil Sjolander
c28429efc8 Dont override flexShrink, flexGrow, and flexBasis with shorthand flex
Summary: when setting both flex and flexGrow then flexGrow should override flex even though flex was setter after.

Reviewed By: gkassabli

Differential Revision: D4064696

fbshipit-source-id: db2d4b8e60209f0a9eed6794a167b85e453be41c
2016-10-24 03:37:48 -07:00
Emil Sjolander
ab4ce535b9 Forward gLogger output to adb on android platforms
Summary: use android/log on android platforms

Reviewed By: splhack

Differential Revision: D4064619

fbshipit-source-id: de23e72844e25106d0db756064f5699959f45ed2
2016-10-23 11:07:43 -07:00