Advertisement
wellthatsucks

Untitled

Sep 11th, 2018
131
0
Never
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 9.71 KB | None | 0 0
  1. <Mathieu_Du> Hey, I'm trying to fix the test for raster-source when builddir != srcdir, it fails because of https://github.com/mrobinson/cairo/blob/master/test/raster-source.c#L90
  2. <Mathieu_Du> const char *png_filename = "png.png";
  3. <Mathieu_Du> problem is if instead of a static string here I allocate a string ('%s/png.png' % ctx->srcdir basically), I don't know where to free it so that the recording tests don't break
  4. <__tim> via atexit() maybe?
  5. <Mathieu_Du> hm that could work yes
  6. <__tim> or at the end of draw() ?
  7. <Mathieu_Du> no, end of draw is a problem, because it gets used after that with recording tests
  8. <Mathieu_Du> it is passed as the closure argument to "png_acquire"
  9. <Mathieu_Du> but atexit + static global should work appropriately
  10. <__tim> ah, fun
  11. <Mathieu_Du> tons yes ^^
  12. <ebassi> Mathieu_Du: Why can't you just leak it?
  13. <ebassi> It's a test, isn't it?
  14. <ebassi> I mean, it's a one-off allocation, not really a leak
  15. <Mathieu_Du> ebassi, given valgrind is mentioned as a potential test wrapper I'd say leaking in a test isn't an option :)
  16. <ebassi> You can use a suppression file for one-off allocations
  17. <Mathieu_Du> that's an option, I don't like it :)
  18. <ebassi> Using atexit() to free a single string looks like a massive pain; it's not even guaranteed to be portable
  19. <ebassi> You could attach it as user data to something like a cairo_surface_t or a cairo_t
  20. <ebassi> Anyway, it's just drive-by comments
  21. <Mathieu_Du> cairo uses atexit in other places
  22. * jghali has quit (Quit: Leaving)
  23. <Mathieu_Du> eg cairo-trace.c
  24. <Mathieu_Du> and the problem is that it *is* attached as user data to the raster source
  25. <Mathieu_Du> and it's not obvious to me in which callback I could safely dispose of it, that's why I was asking here in the first place :)
  26. <Mathieu_Du> ebassi, ooc what are the portability concerns of atexit ?
  27. * KaL has quit (Quit: Bye)
  28. <ebassi> Mathieu_Du: It's POSIX, not C standard; it's also weird when it comes to dlopened modules
  29. <ebassi> Mathieu_Du: We have a wrapper for it in GLib, and it's been deprecated years ago: https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-atexit
  30. <ebassi> It's also basically undefined for libraries; in this case, for a test, it may be fine — but, then again, you could also use an __attribute__((destructor))
  31. <Mathieu_Du> hm noted
  32. <Mathieu_Du> I'll file an issue anyway, someone will probably know of a better solution altogether
  33. * dreamon_ (~dreamon@unaffiliated/dreamon) has joined #cairo
  34. * xjuan_ (~xjuan@host72.201-253-200.telecom.net.ar) has joined #cairo
  35. * xjuan has quit (Ping timeout: 240 seconds)
  36. * lazka (~lazka@213.147.165.99) has joined #cairo
  37. * lazka has quit (Client Quit)
  38. <Mathieu_Du> hrm, --enable-qt=yes doesn't pass because of a patch merged in 2015 oO
  39. <Mathieu_Du> s/pass/build/
  40.  
  41. [snip]
  42.  
  43. <psychon> Mathieu_Du: cairo_pattern_set_user_data provides you a way to free something when the pattern is destroyed, just do static cairo_user_data_key some_key; c_p_s_u_d(pattern, &some_key, data_to_free, free);
  44. <Mathieu_Du> psychon, in what callback do you actually free that user data ?
  45. <psychon> just do exactly what I wrote above and it is freed when the pattern is destroyed
  46. <psychon> and the callback here is free() itself which (AFAIR) happens to have just the right signature to be used here
  47. <psychon> so: static cairo_user_data_key_t some_key; cairo_pattern_set_user_data(your_pattern, &some_key, the_data_you_want_to_free_when_the_pattern_is_destroyed, free);
  48. <Mathieu_Du> I see :)
  49. <Mathieu_Du> lemme try
  50. <psychon> originally this user data API is meant as a kind of hashmap that you can look up again later, but it always had the ability to free the data when it becomes unreachable
  51. <psychon> alternatively, just use a static char buffer[PATH_MAX]; and ignore that this breaks for too long paths ;-)
  52. <Mathieu_Du> heh, might as well do it the right way (tm)
  53. <Mathieu_Du> tnx
  54. <Mathieu_Du> psychon, same deal as with the finish callback
  55. <Mathieu_Du> the filename gets freed then the acquire function gets called again and things break
  56. <Mathieu_Du> that's when the tests use the recording surface fwiw
  57. <Mathieu_Du> hm, I fixed compile issues with QT surfaces but tests crash and burn :(
  58.  
  59. [snip]
  60.  
  61. <Mathieu_Du> psychon, that's what it does, I don't think that's a bug
  62. <Mathieu_Du> pretty easy to reproduce btw
  63. <Mathieu_Du> drm surface not building on arch linux
  64. <Mathieu_Du> fixing the includes just reveals a crapton more issues
  65. <Mathieu_Du> the drm surface is broken since at least 2011 oO
  66. <Mathieu_Du> bryce, nice to see you are addressing the whitespace issues, but wouldn't it be better to stop using mixed spaces and tabs ?
  67. <Mathieu_Du> and probably run the whole thing through indent and add a pre-commit hook ^^
  68. <bryce_> Mathieu_Du, probably, but a lot of this is historical, done by various other people.
  69. * hjdskes has quit (Ping timeout: 240 seconds)
  70. <Mathieu_Du> I was referring to https://gitlab.com/cairo/cairo/commit/85fe4deee4bb01a211924d92263540e93cd9e5db specifically :)
  71. * hjdskes (~hjdskes@ip-213-127-12-91.ip.prioritytelecom.net) has joined #cairo
  72. <Mathieu_Du> whatever I suppose, but cairo renders all wrong because of this on my end (iirc freetype or fontconfig had the same mixed-tabs problem)
  73.  
  74. [snip]
  75.  
  76. <Mathieu_Du> hrm, all the gl tests crash for me (FPE)
  77.  
  78. [snip]
  79.  
  80. <Mathieu_Du> Hey bryce_ around ? :)
  81. <bryce_> Mathieu_Du, yep
  82. <Mathieu_Du> \o/
  83. <Mathieu_Du> got a pretty decent meson port :)
  84. <bryce_> oh wow
  85. <Mathieu_Du> just done building it on Windows as well
  86. <Mathieu_Du> cairo's test suite gave me endless amounts of grief ^^
  87. <Mathieu_Du> didn't even attempt to fully run it on Windows
  88. <bryce_> Mathieu_Du, oh I can definitely imagine that! there's a lot of complex machinery in there
  89. <bryce_> Mathieu_Du, congrats, if it builds and runs on linux that's a nice achievement
  90. <Mathieu_Du> It builds and runs and the test suite fails at parity with the autotools build
  91. <Mathieu_Du> best setup I could get was:
  92. <Mathieu_Du> * get ghostscript 9.06 , 9.23 just fails because of a .setlanguagelevel postscript directive
  93. <Mathieu_Du> * run xvfb
  94. <Mathieu_Du> * use the autotools build to regenerate the reference images
  95. <Mathieu_Du> Then run the test suite built with meson against that
  96. <Mathieu_Du> I still got failures after all that, but afaict they were the same as the autotools build
  97. <Mathieu_Du> bryce_, if you want to give the port a try it's at https://github.com/centricular/cairo
  98. <bryce_> thanks
  99. <Mathieu_Du> there are some commits you might want to cherry-pick on that branch btw :)
  100. <Mathieu_Du> Kept them separate from the meson work
  101. <bryce_> yeah the test suite's rendering tests seem to have some dependence on other libraries, so changes in those versions can cause false failures against our reference images
  102. <Mathieu_Du> yes, that test suite seems extremely fragile
  103. <bryce_> I figure we'll have to designate a "standard" platform to run the tests against.
  104. <Mathieu_Du> also I noticed that the reference images had to be regenerated if you changed the build options, specifically I noticed that with xcb
  105. <bryce_> I figure Ubuntu 18.04.0 will be convenient to baseline on. I've set up a box with that on it and run the testsuite, and it's on my todo list to review the discrepancies and update the test images where it's obviously just fuzz from pixman or pango or whatnot
  106. <Mathieu_Du> Note that meson could help with this, though not entirely
  107. <bryce_> I'll be interested in see what needed changed for that, I know the testsuite currently depends on autotools specifics
  108. <Mathieu_Du> for example, I've checked in wrap files for my meson ports of freetype, fontconfig and pixman
  109. * hjdskes has quit (Ping timeout: 240 seconds)
  110. <Mathieu_Du> this can help setting up a stable environment wrt these dependencies at least
  111. <bryce_> I'm suspecting that aside from the testsuite the other important thing is supporting the myriad autoconf options that the current system does... all the weird backends and configurables. Do you have plans to look into those?
  112. * hjdskes (~hjdskes@ip-213-127-12-91.ip.prioritytelecom.net) has joined #cairo
  113. <Mathieu_Du> My port supports nearly all the backends (that aren't broken with autotools as well)
  114. <bryce_> cool. I imagine there's a lot that's just obsolete or unnecessary cruft
  115. <Mathieu_Du> the only two possibly non-broken backends (though I kind of doubt they have been tested in a long time) that I haven't ported are OS/2 and BEOS
  116. <bryce_> I'm not sure about BEOS
  117. <Mathieu_Du> Backends that seem to be broken are QT, drm and cogl
  118. <Mathieu_Du> A few others I have set up but not tested, eg quartz
  119. <Mathieu_Du> bryce_, care to give building it a go? I've only tested on my system thus far :)
  120. <bryce_> there was a set of patches a while back to fix the drm backend
  121. <Mathieu_Du> hm, as far as I could tell it couldn't build since at least 2011
  122. <Mathieu_Du> potentially earlier
  123. <bryce_> Inkscape has a mac port, so OS/2 and/or quartz is going to be important there. Unfortunately the folks working on the Inkscape ports there are all gone now.
  124. <Mathieu_Du> hrm, OS/2 really ?
  125. <Mathieu_Du> re quartz it might work out of the box (or not)
  126. <__tim> OS/2 :D
  127. <Mathieu_Du> last release 2001 :P
  128. <bryce_> http://paste.ubuntu.com/p/VmGH3J8f8V/
  129. <Mathieu_Du> Ah right you need meson from git, I added the dictionary type there recently :)
  130. <bryce_> yeah my meson is pretty old, 0.40.1
  131. <Mathieu_Du> it has no dependency so you can run it directly from a git clone
  132. <__tim> should probably add the meson version requirement to project() then :)
  133. <Mathieu_Du> true that, though I'm not sure if I can specify "git master" :P
  134. * hjdskes has quit (Ping timeout: 248 seconds)
  135. * hjdskes (~hjdskes@ip-213-127-12-91.ip.prioritytelecom.net) has joined #cairo
  136. <__tim> > 0.46.99 perhaps?
  137. <Mathieu_Du> that could work yes :)
  138. <Mathieu_Du> fixed
  139. <Mathieu_Du> tnx
Advertisement
Add Comment
Please, Sign In to add comment
Advertisement