Skip to content
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

Fix dynamic framework support and fix binary compatibility of SkikoUI… #763

Merged
merged 2 commits into from
Jul 18, 2023
Merged

Fix dynamic framework support and fix binary compatibility of SkikoUI… #763

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a930272
Fix dynamic framework support and fix binary compatibility of SkikoUI…
AlexeyTsvetkov Jul 15, 2023
4961f8a
CR fix: use flatmap
AlexeyTsvetkov Jul 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 46 additions & 16 deletions skiko/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import org.gradle.crypto.checksum.Checksum
import org.jetbrains.compose.internal.publishing.MavenCentralProperties
import org.jetbrains.kotlin.gradle.plugin.KotlinTarget
import org.jetbrains.kotlin.gradle.plugin.mpp.KotlinNativeTarget
import org.jetbrains.kotlin.gradle.tasks.CInteropProcess
import org.jetbrains.kotlin.gradle.tasks.KotlinCompileTool

plugins {
Expand Down Expand Up @@ -427,6 +428,32 @@ kotlin {
}
}

fun configureCinterop(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the property enableCInteropCommonization for something else? (docs doesn't say its purpose clearly)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enableCInteropCommonization is used to tell Kotlin Gradle plugin to commonize multiple CInterop libraries.

So for example, for the following configuration:

kotlin {
    linuxX64 {
        compilations.getByName("main") {
             cinterops.create("foo").apply { ... }
        }
    }
    linuxArm64  {
        compilations.getByName("main") {
             cinterops.create("foo").apply { ... }
        }
    }

    sourceSets {
           val commonMain by getting
           val linuxCommonMain by creating { dependsOn(commonMain) }
           val linuxX64Main by getting { dependsOn(linuxCommonMain) }
           val linuxArm64Main by getting { dependsOn(linuxCommonMain) }
     }
}

cinterop creates two platform specific libraries: cinterop-foo-x64.klib and cinterop-foo-arm64.klib
By default, cinterop-foo declarations would be available only in platform specific source sets.
So consumers would not be able to use foo declarations in linuxCommon source sets.
kotlin.mpp.enableCInteropCommonization=true generates commonized klib with common declarations for linuxCommonMain (and its consumers).

I don't think we need this flag now, since we don't need to commonize any cinterop declarations.

cinteropName: String,
os: OS,
arch: Arch,
target: KotlinNativeTarget,
targetString: String,
linkerOpts: List<String>,
) {
val tasks = target.project.tasks
val taskNameSuffix = joinToTitleCamelCase(os.idWithSuffix(isIosSim = target.isIosSimArm64()), arch.id)
val writeCInteropDef = tasks.register("writeCInteropDef$taskNameSuffix", WriteCInteropDefFile::class.java) {
this.linkerOpts.set(linkerOpts)
outputFile.set(project.layout.buildDirectory.file("cinterop/$targetString/skiko.def"))
AlexeyTsvetkov marked this conversation as resolved.
Show resolved Hide resolved
}
tasks.withType(CInteropProcess::class.java).configureEach {
if (konanTarget == target.konanTarget) {
dependsOn(writeCInteropDef)
}
}
target.compilations.getByName("main") {
cinterops.create(cinteropName).apply {
defFileProperty.set(writeCInteropDef.map { it.outputFile.get().asFile })
}
}
}

fun configureNativeTarget(os: OS, arch: Arch, target: KotlinNativeTarget) {
if (!os.isCompatibleWithHost) return

Expand All @@ -444,24 +471,27 @@ fun configureNativeTarget(os: OS, arch: Arch, target: KotlinNativeTarget) {

val skiaBinDir = "$skiaDir/out/${buildType.id}-$targetString"
val linkerFlags = when (os) {
OS.MacOS -> mutableListOf("-linker-option", "-framework", "-linker-option", "Metal",
"-linker-option", "-framework", "-linker-option", "CoreGraphics",
"-linker-option", "-framework", "-linker-option", "CoreText",
"-linker-option", "-framework", "-linker-option", "CoreServices"
OS.MacOS -> mutableListOfLinkerOptions(
listOfFrameworks("Metal", "CoreGraphics", "CoreText", "CoreServices")
)
OS.IOS -> mutableListOf("-linker-option", "-framework", "-linker-option", "Metal",
"-linker-option", "-framework", "-linker-option", "CoreGraphics",
"-linker-option", "-framework", "-linker-option", "UIKit",
"-linker-option", "-framework", "-linker-option", "CoreText")
OS.Linux -> mutableListOf(
"-linker-option", "-L/usr/lib/x86_64-linux-gnu",
"-linker-option", "-lfontconfig",
"-linker-option", "-lGL",
OS.IOS -> {
val iosFrameworks = listOfFrameworks("Metal", "CoreGraphics", "CoreText", "UIKit")
// list of linker options to be included into klib, which are needed for skiko consumers
// https://github.com/JetBrains/compose-multiplatform/issues/3178
// Important! Removing or renaming cinterop-uikit publication might cause compile error
// for projects depending on older Compose/Skiko transitively https://youtrack.jetbrains.com/issue/KT-60399
configureCinterop("uikit", os, arch, target, targetString, iosFrameworks)
mutableListOfLinkerOptions(iosFrameworks)
}
OS.Linux -> mutableListOfLinkerOptions(
"-L/usr/lib/x86_64-linux-gnu",
"-lfontconfig",
"-lGL",
// TODO: an ugly hack, Linux linker searches only unresolved symbols.
"-linker-option", "$skiaBinDir/libsksg.a",
"-linker-option", "$skiaBinDir/libskshaper.a",
"-linker-option", "$skiaBinDir/libskunicode.a",
"-linker-option", "$skiaBinDir/libskia.a"
"$skiaBinDir/libsksg.a",
"$skiaBinDir/libskshaper.a",
"$skiaBinDir/libskunicode.a",
"$skiaBinDir/libskia.a"
)
else -> mutableListOf()
}
Expand Down
27 changes: 27 additions & 0 deletions skiko/buildSrc/src/main/kotlin/WriteCInteropDefFile.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import org.gradle.api.DefaultTask
import org.gradle.api.file.RegularFileProperty
import org.gradle.api.provider.ListProperty
import org.gradle.api.tasks.Input
import org.gradle.api.tasks.OutputFile
import org.gradle.api.tasks.TaskAction

abstract class WriteCInteropDefFile : DefaultTask() {
@get:Input
abstract val linkerOpts: ListProperty<String>

@get:OutputFile
abstract val outputFile: RegularFileProperty

@TaskAction
fun run() {
val outputFile = outputFile.get().asFile
outputFile.parentFile.mkdirs()

outputFile.bufferedWriter().use { writer ->
val linkerOpts = linkerOpts.get()
if (linkerOpts.isNotEmpty()) {
writer.appendLine("linkerOpts=${linkerOpts.joinToString(" ")}")
}
}
}
}
14 changes: 14 additions & 0 deletions skiko/buildSrc/src/main/kotlin/utils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,17 @@ fun Task.projectDirs(vararg relativePaths: String): List<Directory> {
val projectDir = project.layout.projectDirectory
return relativePaths.map { path -> projectDir.dir(path) }
}

fun listOfFrameworks(vararg frameworks: String): List<String> =
frameworks.flatMap { listOf("-framework", it) }

fun mutableListOfLinkerOptions(options: Collection<String>): MutableList<String> =
AlexeyTsvetkov marked this conversation as resolved.
Show resolved Hide resolved
ArrayList<String>(options.size * 2).also { result ->
options.forEach { option ->
result.add("-linker-option")
result.add(option)
}
}

fun mutableListOfLinkerOptions(vararg options: String): MutableList<String> =
mutableListOfLinkerOptions(options.toList())
15 changes: 14 additions & 1 deletion skiko/src/iosMain/kotlin/org/jetbrains/skiko/SkikoUIView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import platform.darwin.NSInteger
import kotlin.math.max
import kotlin.math.min

/*
TODO: remove org.jetbrains.skiko.objc.UIViewExtensionProtocol after Kotlin 1.8.20
https://youtrack.jetbrains.com/issue/KT-40426
*/
@Suppress("CONFLICTING_OVERLOADS")
@ExportObjCClass
class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
Expand All @@ -30,11 +34,20 @@ class SkikoUIView : UIView, UIKeyInputProtocol, UITextInputProtocol {
private var _inputDelegate: UITextInputDelegateProtocol? = null
private var _currentTextMenuActions: TextActions? = null

// merging two constructors might cause a binary incompatibility, which will result in a unclear linking error,
// if a project using newer compose depends on an older compose transitively
// https://youtrack.jetbrains.com/issue/KT-60399
constructor(
skiaLayer: SkiaLayer,
frame: CValue<CGRect> = CGRectNull.readValue(),
pointInside: (Point, UIEvent?) -> Boolean = {_,_-> true }
) : this(skiaLayer, frame, pointInside, skikoUITextInputTrains = object : SkikoUITextInputTraits {})

constructor(
skiaLayer: SkiaLayer,
frame: CValue<CGRect> = CGRectNull.readValue(),
pointInside: (Point, UIEvent?) -> Boolean = {_,_-> true },
skikoUITextInputTrains: SkikoUITextInputTraits = object : SkikoUITextInputTraits {},
skikoUITextInputTrains: SkikoUITextInputTraits
) : super(frame) {
this.skiaLayer = skiaLayer
_pointInside = pointInside
Expand Down