Skip to content

Fix skewed borders and text-decoration thickness #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Feb 27, 2022
Merged

Fix skewed borders and text-decoration thickness #10

merged 3 commits into from
Feb 27, 2022

Conversation

lonemadmax
Copy link

drawLine arguments are the corners of the rectangle containing the border piece.
drawLinesForText was not setting the pen width.

Feel free to squash after review.

pulkomandy
pulkomandy reviewed Feb 26, 2022
View reviewed changes
@lonemadmax
Restore pen size after drawing a stroked rect
868991f
@lonemadmax
Set pen size for text lines
90e7e35 
Fixes part of https://dev.haiku-os.org/ticket/17611
@lonemadmax
Fix skewed styled borders
78202a8 
The arguments for GraphicsContext::drawLine are the top left and bottom
right limits of the rectangle for that line considering its thickness,
not the start and end points.

Fixes the other part of https://dev.haiku-os.org/ticket/17611
@lonemadmax
Copy link
Author

I'll also try to make dotted and dashed styles work for text decorations; not sure if you'll prefer to wait and merge it all together or separate it as that's not part of the bugfix.

@pulkomandy
Copy link
Member

ok then, I understand where you're going and it's fine to merge this for now.

Do you plan to implement dotted and dashed styles in webkit side by drawing a lot of short lines? Wouldn't it be more efficient to add a BView API for it and implement it on app_server side?

@pulkomandy pulkomandy merged commit 30f313d into haiku:haiku Feb 27, 2022
@lonemadmax
Copy link
Author

I'm really incompetent at API design or even tweak, and touching the app_server at that level is still scary for me. I may have a look at it, though, but I can't promise anything.

I was just going to remove the TODO about using line arrays without consideration of performance, and dots would still be squares instead of circles. I guess that would not be very efficient for long runs of text, and it is very probably worse than current code for the most common case of widths.size() == 2.

And then there's also the middle ground of using a B_MIXED_COLORS pattern like with the other objects. That one would be easy without a lot of change.

@lonemadmax lonemadmax deleted the fix-17611 branch February 27, 2022 19:13
pulkomandy pushed a commit that referenced this pull request Nov 22, 2022
@hyjorc1
Fix TypeError in parameter destructuring in async function should be …
29265f9 
…a rejected promise

https://bugs.webkit.org/show_bug.cgi?id=247785
rdar://102325201

Reviewed by Yusuke Suzuki.

Rest parameter should be caught in async function. So, running this
JavaScript program should print "caught".
```
async function f(...[[]]) { }
f().catch(e => print("caught"));
```

V8 (used console.log)
```
$ node input.js
caught
```

GraalJS
```
$ js input.js
caught
```

https://tc39.es/ecma262/#sec-async-function-definitions
...
AsyncFunctionDeclaration[Yield, Await, Default] :
    async [no LineTerminator here] function BindingIdentifier[?Yield, ?Await] ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
    [+Default] async [no LineTerminator here] function ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }

AsyncFunctionExpression :
    async [no LineTerminator here] function BindingIdentifier[~Yield, +Await]opt ( FormalParameters[~Yield, +Await] ) { AsyncFunctionBody }
...

According to the spec, it indicates `FormalParameters` is used for Async
Function, where `FormalParameters` can be converted to `FunctionRestParameter`.

https://tc39.es/ecma262/#sec-parameter-lists
...
FormalParameters[Yield, Await] :
    [empty]
    FunctionRestParameter[?Yield, ?Await]
    FormalParameterList[?Yield, ?Await]
    FormalParameterList[?Yield, ?Await] ,
    FormalParameterList[?Yield, ?Await] , FunctionRestParameter[?Yield, ?Await]
...

And based on RS: EvaluateAsyncFunctionBody, it will invoke the promise.reject
callback function with abrupt value ([[value]] of non-normal completion record).

https://tc39.es/ecma262/#sec-runtime-semantics-evaluateasyncfunctionbody
...
2. Let declResult be Completion(FunctionDeclarationInstantiation(functionObject, argumentsList)).
3. If declResult is an abrupt completion, then
    a. Perform ! Call(promiseCapability.[[Reject]], undefined, « declResult.[[Value]] »).
...

In that case, any non-normal results of evaluating rest parameters should be
caught and passed to the reject callback function.

To resolve this problem, we should allow the emitted RestParameterNode be wrapped
by the catch handler for promise. However, we should remove `m_restParameter` and
emit rest parameter byte code in `initializeDefaultParameterValuesAndSetupFunctionScopeStack`
if we can prove that change has no side effect. In that case, we can only use one
exception handler.

Current fix is to add another exception handler. And move the handler byte codes to
the bottom of code block in order to make other byte codes as much compact as possible.

Input:
```
async function f(arg0, ...[[]]) { }
f();
```

Dumped Byte Codes:
```
...

bb#2
Predecessors: [ #1 ]
[  20] mov                dst:loc9, src:<JSValue()>(const0)
...

bb#3
Predecessors: [ #2 ]
[  29] get_rest_length    dst:loc11, numParametersToSkip:1
...

bb#12
Predecessors: [ #8 #9 #10 ]
[ 138] new_func_exp       dst:loc10, scope:loc4, functionDecl:0
...

bb#13
Predecessors: [ ]
[ 170] catch              exception:loc10, thrownValue:loc8
[ 174] jmp                targetLabel:8(->182)
Successors: [ #15 ]

bb#14
Predecessors: [ #7 #11 ]
[ 176] catch              exception:loc10, thrownValue:loc8
[ 180] jmp                targetLabel:2(->182)
Successors: [ #15 ]

bb#15
Predecessors: [ #13 #14 ]
[ 182] mov                dst:loc12, src:Undefined(const1)
...

Exception Handlers:
	 1: { start: [  20] end: [  29] target: [ 170] } synthesized catch
	 2: { start: [  29] end: [ 138] target: [ 176] } synthesized catch
```

* JSTests/stress/catch-rest-parameter.js: Added.
(throwError):
(shouldThrow):
(async f):
(throwError.async f):
(throwError.async let):
(async let):
(x.async f):
(x):
(async shouldThrow):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::BytecodeGenerator):
(JSC::BytecodeGenerator::initializeDefaultParameterValuesAndSetupFunctionScopeStack):
* Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:

Canonical link: https://commits.webkit.org/256864@main
pulkomandy pushed a commit that referenced this pull request Mar 5, 2023
@Constellation
[JSC] Support wasm frames in btjs
560bfa8 
https://bugs.webkit.org/show_bug.cgi?id=251063
rdar://104585575

Reviewed by Mark Lam and Justin Michaud.

This patch enhances CallFrame::dump to support wasm frames in btjs stacktrace.
The example is as follows.

    frame #0: 0x00000001035fca78 JavaScriptCore`JSC::functionBreakpoint(globalObject=0x000000012f410068, callFrame=0x000000016fdfa9d0) at JSDollarVM.cpp:2273:9 [opt]
    frame #1: 0x000000010ec44204 0x10eccc5dc
    frame #2: 0x000000010eccc5dc callback#Dwaxn6 [Baseline bc#50](Undefined)
    frame #3: 0x000000010ec4ca84 wasm-stub [WasmToJS](Wasm::Instance: 0x10d29da40)
    frame #4: 0x000000010ed0c060 <?>.wasm-function[1] [OMG](Wasm::Instance: 0x10d29da40)
    frame #5: 0x000000010ed100d0 jsToWasm#CWTx6k [FTL bc#22](Cell[JSModuleEnvironment]: 0x12f524540, Cell[WebAssemblyFunction]: 0x10d06a3a8, 1, 2, 3)
    frame #6: 0x000000010ec881b0 #D5ymZE [Baseline bc#733](Undefined, Cell[Generator]: 0x12f55c180, 1, Cell[Object]: 0x12f69dfc0, 0, Cell[JSLexicalEnvironment]: 0x12f52cee0)
    frame #7: 0x000000010ec3c008 asyncFunctionResume#A4ayYg [LLInt bc#49](Undefined, Cell[Generator]: 0x12f55c180, Cell[Object]: 0x12f69dfc0, 0)
    frame #8: 0x000000010ec3c008 promiseReactionJobWithoutPromise#D0yDF1 [LLInt bc#25](Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame #9: 0x000000010ec80ec0 promiseReactionJob#EdShZz [Baseline bc#74](Undefined, Undefined, Cell[Function]: 0x12f44f3c0, Cell[Object]: 0x12f69dfc0, Cell[Generator]: 0x12f55c180)
    frame #10: 0x000000010ec3c728
    frame #11: 0x0000000103137560 JavaScriptCore`JSC::Interpreter::executeCall(JSC::JSGlobalObject*, JSC::JSObject*, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) [inlined] JSC::JITCode::execute(this=<unavailable>, vm=<unavailable>, protoCallFrame=<unavailable>) at JITCodeInlines.h:42:38 [opt]
    frame #12: 0x0000000103137524 JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, lexicalGlobalObject=<unavailable>, function=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) at Interpreter.cpp:1093:27 [opt]
    frame #13: 0x000000010349d6d0 JavaScriptCore`JSC::runJSMicrotask(globalObject=0x000000012f410068, identifier=(m_identifier = 81), job=JSValue @ x22, argument0=JSValue @ x26, argument1=JSValue @ x25, argument2=<unavailable>, argument3=<unavailable>) at JSMicrotask.cpp:98:9 [opt]
    frame #14: 0x00000001039dfc54 JavaScriptCore`JSC::VM::drainMicrotasks() (.cold.1) at VM.cpp:0:9 [opt]
    frame #15: 0x00000001035e58a4 JavaScriptCore`JSC::VM::drainMicrotasks() [inlined] JSC::MicrotaskQueue::dequeue(this=<unavailable>) at VM.cpp:0:9 [opt]
    frame #16: 0x00000001035e5894 JavaScriptCore`JSC::VM::drainMicrotasks(this=0x000000012f000000) at VM.cpp:1255:46 [opt]
    ...

* Source/JavaScriptCore/interpreter/CallFrame.cpp:
(JSC::CallFrame::dump const):

Canonical link: https://commits.webkit.org/259262@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pulkomandy pulkomandy pulkomandy left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lonemadmax @pulkomandy