openhab-android
openhab-android copied to clipboard
openhab
Add support for external fonts and CSS files in SVG images
Is your feature request related to a problem? Please describe.
Problem: Rapidly change the style for a whole collection of SVG images/icons by having a shared CSS file that is included in each SVG (e.g. change color, dark/light mode switch).
The included SVG rendering library AndroidSVG supports external fonts and CSS files (e.g. SVG @import "style.css") but the user application should override/implement the SVGExternalFileResolver class. Documentation is here and in official SVG pages
Describe the solution you'd like
Extend SVGExternalFileResolver with resolveFont and resolveCSSStyleSheet e.g. in ExtensionFuncs.kt. Happy to give it a shot (although I am new to Kotlin) and if it works submit a PR
Describe alternatives you've considered
Include all styling in each SVG (versus a shared CSS file of all SVG).
Additional context
Add any other context or screenshots about the feature request here.
Chart with imported CSS and one without (for testing) chart_extCSS.zip chart.zip
PS. These SVG charts also don't render properly on the AndoridSVG library, but that is an issue with that library and I'm filing a PR with them (or find a workaround)
Happy to give it a shot (although I am new to Kotlin) and if it works submit a PR
:+1: Go for it!
Some implementation notes:
- Since we can simply extend the resolver class, we should do so instead of using extension functions.
- The source CSS and font files are simply put into the images directory on the server?
- The class constructor likely will need a reference to an
HttpClientinstance to fetch the images from - Fetching bitmaps and CSS files is easy, but
Typefaceinstances can't be created from an InputStream. Probably need to save them to the app's cache directory and construct them from there (and skip download if file already exists) - There's a huge caveat due to bad SVG library design: we only can have a resolver singleton, but we have two possible HTTP clients (active and remote server) ... I'm not sure how to solve this in a non-hacky way. A proper library design would allow passing in a given resolver when calling the SVG factory methods.
Some thoughts on this:
The included SVG rendering library AndroidSVG supports external fonts and CSS files (e.g. SVG @import "style.css")
IMO you should open an issue in the https://github.com/openhab/openhab-webui repo as well to agree on a path for the imported files.
Rapidly change the style for a whole collection of SVG images/icons
IMO for icons there a better ways to quickly change the whole set:
- Quick and dirty: Rename the folder
classictoclassic.originside theiconsdirectory on the server. Then create a symlinkclassic -> classic.origand a new folder, e.g.custom-light. Now you can change the icon set by changing the target of the symlink. - With a bit effort: Create icon packs for each collection you have and contribute it to openHAB :) If there's more than just the default icon pack, I'll implement #785.
The source CSS and font files are simply put into the images directory on the server?
Yes, CSS would typically be in same folder (or subfolder) as SVG. With respect to the 3 external types of resources:
- CSS: the primary use case --> that is what I would try to implement since the most useful and simple
- fonts: probably not needed. The challenge I was facing (see picture in issue #2021) is that the default SVG font is not the HabDroid default font but something like Times Roman and that does not look nice. So what I really wanted to figure out is whether it's possible to just to pick up an already loaded font. Something like: if requested font is already loaded (e.g. Roboto), return it, if not return the default (HabDroid) font.
- bitmaps: don't see the use case for this (yet) since PNG/JPEG icons can be used if fixed size icons are desired. The whole point of SVG IMHO is flexible scaling with client display estate for icons and (now) charts.
IMO for icons there a better ways to quickly change the whole set: Rename the folder
Renaming the directory (icons-light/icons-dark to icons) is what I currently do, but it still requires every SVG to be edited individually (for fill colors etc.) and then duplicated. The idea was to be able to just rename the CSS file instead (theme-light.css/theme-dark.css --> theme.css) if @import would be implemented since I found that the HabDroid app already passes the theme (e.g. theme = dark/light) e.g. as part of the URL arguments when it request a chart (which is really great!)
Since we can simply extend the resolver class, we should do so instead of using extension functions. The class constructor likely will need a reference to an HttpClient instance to fetch the images from
Exactly what I had in mind :-)
There's a huge caveat due to bad SVG library design: we only can have a resolver singleton, but we have two possible HTTP clients (active and remote server) ... I'm not sure how to solve this in a non-hacky way. A proper library design would allow passing in a given resolver when calling the SVG factory methods.
Technically the SVG spec allows for a full URL path name (e.g. to a foreign server) but that would introduce all kind of issues (e.g CORS) and that is also way beyond the use case. So I had in mind to limit the @import to the same server that is providing the SVG (pick up that connection and generate a HTTPClient for it, if somehow possible).
pick up that connection and generate a HTTPClient for it, if somehow possible
That's the problem. One could have two threads loading SVGs simultaneously, e.g. when a notification with SVG icon arrives (loaded from remote connection, usually myopenhab) while loading sitemap data (loaded from active connection, which may be local or remote depending on active network type) ... the problem is that in the resolver, you have no reference to the caller which triggers the request.
pick up that connection and generate a HTTPClient for it, if somehow possible
With the attached patch I seem to be able to pick up the url for all bitmaps from the appropriate connection. It hits only 3 locations and requires really minor, straightforward changes. It took just 5 minutes and I did not experience or see any bad design IMHO (rather the opposite)...
From there I assume I can now create a HTTPClient using
httpClient.buildUrl(url)
From there I assume I can now create a HTTPClient using httpClient.buildUrl(url)
No. You can ask an existing HttpClient object to convert a (potentially) relative URL to an absolute URL by calling that method on it. In your case, you don't have a HttpClient object reference, which is the problem I mentioned. It also doesn't help to pass the client reference to InputStream.svgToBitmap, since the resolver is a singleton without any reference to the SVG.getFromInputStream() call, and multiple calls to InputStream.svgToBitmap can happen in parallel.
Ah, excuse my slow brain. Covid19 infection has done little to improve it. Back to square one...
This patch is getting me a HTTPClient, and the baseURL seems OK.
Basically it just extends class HttpResult with the HTTPClient, so it can be passed it on.
Probably the changes in ItemUpdateWidget.kt (fetchAndSetIcon) are not correct (might not need a new connection from the factory), but I was unable to trigger (a breakpoint in) that function to see if I could solve it in a simpler fashion. Don't know how to trigger that code...
Sorry if I'm testing your patience...
This patch is getting me a HTTPClient, and the baseURL seems OK. Basically it just extends class HttpResult with the HTTPClient, so it can be passed it on.
So now you got the HttpClient in the SVG conversion routine ... but we need it in the resolver class instance. How do you get it there? ;-) Creating a new resolver instance every time (with the client as constructor parameter) doesn't work as mentioned before due to the static setter...
Sorry if I'm testing your patience...
You don't, don't worry :-)
@ewaldc Can you upload the code to a new branch in your fork and create a draft PR?
This compiles correctly and register SVGExternalFileResolver properly. If I understood correctly, the init function of an object is executed only once and the debugger seems to confirm this.
But the @import does not hit breakpoint. Could be an issue with SVGAndroid or my limited Kotlin knowledge...
private val svgResolver: SVGExternalFileResolver = object : SVGExternalFileResolver() {
override fun resolveFont(fontFamily: String, fontWeight: Int, fontStyle: String?): Typeface? {
return null
}
override fun resolveImage(filename: String?): Bitmap? {
return try {
null
} catch (e1: IOException) {
null
}
}
override fun resolveCSSStyleSheet(url: String?): String? {
return try { // --> breakpoint here
null
} catch (e1: IOException) {
null
}
}
init {
SVG.registerExternalFileResolver(this)
}
}
This compiles correctly and register SVGExternalFileResolver properly.
Ack, but how do you get the HttpClient for a specific SVG-to-bitmap transformation into that instance? ;-)
As for the breakpoint, the try-catch might become optimized away. I'd try a log statement there.
One more try:
open class SVGClient {
var client: HttpClient? = null
companion object : SVGExternalFileResolver() {
init {
SVG.registerExternalFileResolver(this)
}
var client: HttpClient? = null
override fun resolveFont(fontFamily: String, fontWeight: Int, fontStyle: String?): Typeface? {
return null
}
override fun resolveImage(filename: String?): Bitmap? {
return try {
null
} catch (e1: IOException) {
null
}
}
override fun resolveCSSStyleSheet(url: String?): String? {
return try {
Log.d(Util.TAG, "SVG bitmapURL: $client") // --> breakpoint here
null
} catch (e1: IOException) {
null
}
}
}
}
private val svgResolver = object : SVGClient() {}
Now I can set the HTTPClient in fun InputStream.svgToBitmap
svgResolver.client = client
val svg = SVG.getFromInputStream(this)
But still no breakpoint being hit. Had to file a defect in SVGAndroid as it does not render fill-opacity properly, so I'll probably need to see where it goes wrong in the code...
Now I can set the HTTPClient in fun InputStream.svgToBitmap
See, that's the ugly part that I do not want. Assume you have two conversions running simultaneously, client will be that of the second conversion and (incorrectly) be used also by callbacks for the first conversion.
I'll open an issue against the lib, I guess.
Tried several solutions but could not find a good way to get around the multi-thread issue besides using a shared mutex around these two statements. The callback should be made context aware as you suggested. Even in good old Linux C kernel all callbacks have context (aka void *). Learned quite a bit of Kotlin today :-).
In the meantime, I found out why @import was not triggering the callback. There is a defect in the CSS parser. I will file an issue report (and perhaps a PR as I found where the error is)
Can't see how to fix BigBadaboom/androidsvg#199 with a totally static library. Will require the instantiation of a SVG object IMHO to store the context (or unique ID of some kind) otherwise it will be needed to pass that context to all parser functions as the callback happens deep in the parser... In that case one can also register the callback for each instance, which means the API does not need to change. Openhab-android (I'm learning!) can then manage a list/pool of objects containing matching SVG object, HTTPClient and callback handler.
From fun InputStream.svgToBitmap we might then request the svg object that matches the current client. I noticed though that there might be multiple threads (ConnectionFactory?) for the same server (do page refreshes happen asynchronously?) so might to manage a pool (2?) of those objects per OH server (= different baseURL in HTTPClient?) .
val svg = svgResolver.getSVG(client) // Obtain SVG object from pool
...
svgResolver.setSVG(null) // release SVG object from pool
Filed a defect for @import issue
Can't see how to fix BigBadaboom/androidsvg#199 with a totally static library. Will require the instantiation of a SVG object IMHO to store the context (or unique ID of some kind) otherwise it will be needed to pass that context to all parser functions as the callback happens deep in the parser...
Yes, it'll be needed to add this to all parser methods, or alternatively, make parser (or its interface) public and allow setting the resolver on that. Setting the resolver on the SVG instance only would help for animated SVGs (I guess?), not for stuff needed during parsing. IMHO setting it on the parser and inheriting it to the SVG instance from there makes most sense to me.
Openhab-android (I'm learning!) can then manage a list/pool of objects containing matching SVG object, HTTPClient and callback handler.
The SVG object essentially equals the image, I don't think we want to track another list of images used throughout the app (we already have the bitmap cache).
From fun InputStream.svgToBitmap we might then request the svg object that matches the current client.
There might (and likely will) be more than one SVG image per client.
I noticed though that there might be multiple threads (ConnectionFactory?)
ConnectionFactory isn't related to that. The threads come from Kotlin coroutines ... check for withContext(Dispatchers.IO) in HttpClient, everything in those blocks will execute on a separate thread pool.
for the same server (do page refreshes happen asynchronously?)
An incoming SSE event ultimately triggers e.g. an image refresh, which happens asynchronously, yes.
so might to manage a pool (2?) of those objects per OH server (= different baseURL in HTTPClient?) .
You lost me ... what do you mean here?
I now have my own fork of androidsvc published in jitpack.io so I can fix defects and test with OH-android build. Already fixed the two issues I submitted... At least I can now test things before submitting a PR. Maybe implement BigBadaboom/androidsvg#199 tomorrow...
Finally, it's working but since it requires a (minor) fix to androidsvg, I can't just provide a PR for just OH-android only. It's running here I had to extend the HttpClient with a synchronousGet since one can not suspend and override a Java class that was not designed as "suspendable", but it's reasonably clean except for not being thread-safe. All the colors for the labels are in "chart.css".
I had to extend the HttpClient with a synchronousGet since one can not suspend and override a Java class that was not designed as "suspendable", but it's reasonably clean except for not being thread-safe.
One actually can ;-) Just enclose the call in an runBlocking block.
Implemented all optimizations you suggested and fixed indentation. runBlocking works as a charm (learning all the time!). It's a much smaller and cleaner change now:
- no extension of HttpClient
- connection object fetched only once and reused in ItemUpdateWidget.kt
- baseUrl remains a private variable in HttpClient client
Could runBlocking also be used to protect the two lines
SVGClient.client = client
val svg = SVG.getFromInputStream(this)
from potential client mismatch in case both local and remote connection threads are active (as a temporary solution until #199 is solved) ?
The one thing I still haven't figured out (but also not really investigated) is why in the demo the weather chart is requested (and thus painted) two times. First I thought that this was due to a page refresh while I was single stepping through the debugger, but it happens also in regular run mode. I happens in the regular beta client too, so it's not something caused by the changes.
URI: /chart - METHOD: HTTP_GET - ARGUMENTS: groups = Weather_Chart; dpi = 640; period = h; random = 1570696976; theme = dark; w = 1440; h = 720;
EDIT: main threads 2 and 3 are both processing the SVG in parallel
Working on fixing the two remaining issues of androidsvg (during lunch time as I resumed work now after Covid infection)
@ewaldc Can you give this branch a try? It uses this branch of androidsvg. If it works, I'll submit the latter as PR to androidsvg.
I think it's missing the fix for resolving external CSS files? Without this fix AndroidSVG will never even call the (now modified) resolver :-)
Well, yes, since I don't want to mix changes for separate PRs and that fix deserves a separate commit.
I've pushed a new 'test' branch including your commit to my androidsvg repo and switched the 'svg-css' branch of the app repo to that androidsvg branch.
Understood, the only challenge is that without this change, SVG Android will not invoke externalFileResolver.resolveCSSStyleSheet(file); since the mediaList will be of size zero. What I can try is to change my copy of your OH Android to invoke the SVG parser so that a mediaLists exists and then change the SVG chart so that it contains a mediaList that matches with the one in the code...
There is one thing I don't understand: mobile/build.gradle contains
compile 'com.github.BigBadaboom:androidsvg:418cf676849b200cacf3465478079f39709fe5b1'
Will that result in your fork of SVGAndroid ? I'm not so good with Gradle, always when I think I understand it a little it does something to throw me off again...
There is one thing I don't understand: mobile/build.gradle contains compile 'com.github.BigBadaboom:androidsvg:418cf676849b200cacf3465478079f39709fe5b1' Will that result in your fork of SVGAndroid ?
No, that one will use upstream. That's why I changed that in my branch. The SHA1 referenced there (8dc61c0...) is the same as in the test branch in my androidsvg fork
Oops, didn't notice Android Studio reset the branch to Master after a crash. Did not notice :-( It's OK now!
Congratulations, working to perfection !
There is of course still the Android SVG defect with fill-opacity, but include of CSS works fine. The one thing I still don't understand is why it loads the chart (and now also the css file) twice. This happens with both a jpg and an svg chart. I noticed it first in the logcat when I added a log line where the CSS external resolver was called. I'm running the standard OH2 demo, and tested with both chart.jpg and chart.svg.
Following Logcats are taken from the standard OH2 demo when the user enters the "Outside Temperature" page with pageid 0100: Logcat with official OH22.5.5 --> png charts Logcat with my OH2 server for ESP --> svg charts
It only happens for the chart image, not for the icons.
The other interesting observation is that the chart is first requested with avoidCache true and immediately thereafter with avoidCache false but it seems that the HttpClient is ignoring this and fetches the chart image twice anyway:
2020-06-03 09:36:16.414 2272-2272/org.openhab.habdroid.beta I/WidgetImageView: Refreshing image at http://192.168.1.20:8080/chart?groups=Weather_Chart&dpi=560&period=h&random=1421411947&theme=dark&w=1440&h=720, avoidCache true
2020-06-03 09:36:16.414 2272-2272/org.openhab.habdroid.beta I/WidgetImageView: Refreshing image at http://192.168.1.20:8080/chart?groups=Weather_Chart&dpi=560&period=h&random=1421411947&theme=dark&w=1440&h=720, avoidCache false
Will test the image tomorrow and see what can be done about the fonts as I don't understand why SVG Android somehow does not use the default Android Roboto font. Could not find it directly in the code...